Comment 5 for bug 1374594

Revision history for this message
Juha Jeronen (juha-jeronen) wrote :

Hmm, the new code looks very good.

This leaves just two more potential issues:

1) Enabling texpix_txaux by checking the presence of specifically the CartoonInk filter seems rather arbitrary.

CartoonInk used to be the only one that needed texpix, but of the new filters, also Scanlines needs to know the pixel size. Additionally, the upcoming new CartoonInk may need also texpix_txdepth depending on its configuration options. Basically, any filter that wants to use whole-pixel offsets in its computations will need texpix for some texture, and for which one, depends on the details of the filter algorithm.

Thus, I think this needs a general mechanism to specify which filters require texpix, and for which textures - similar to the mechanisms you have already added for the texture sampler and texcoord inputs. The specification of exactly which texpix variables are needed by which filters probably belongs into the Filter interface as part of the refactoring.

Note that the various custom textures created for the intermediate stages might have a size different from txcolor; e.g. bloom is rendered into a half-resolution texture. Hence, for generality, texpix must be handled per-texture.

2) Some filters may want to flip some of their input textures along x, along y, or through the view center point. (I missed this possibility in my comment yesterday.)

For example, the first stage of the lens flare filter flips the input color texture around the view center point. This is done in the vshader by modifying the generated texcoords. Operation-count-wise, this is much cheaper than recomputing the texture coordinate in the fshader for each pixel separately.

Currently there are no filters (even among the new ones) that would want to do this for an input of the "main" postprocessing shader, but in the future someone might want to add such a filter.

(In fact, we could easily implement a "mirror" filter by flipping the coordinates? This would be useful as a postprocessing step for rendering textures for camera-based mirrors in the scene.)