Open Bug 1760285 Opened 3 years ago Updated 1 year ago

Rewrite the shader selection logic

Categories

(Core :: Graphics: WebRender, task, P4)

task

Tracking

()

People

(Reporter: nical, Unassigned)

References

(Blocks 1 open bug)

Details

Dealing with shader permutations is surprisingly frustrating:

  • We have to do it in both shade.rs and shader_features.rs
  • There is no generic logic to dealing with shader features, it's all hard coded. For example one could easily assume that requesting ShaderFeatures::ANTIALIASING implies #define WR_FEATURE_ANTIALIASING in the shader, but it's not the case. In fact at the time of writing, that define is mostly useless and we enable most of the AA code paths for all alpha brushes.
  • Adding a shader variant to one or several of the brushes is currently complicated:
    • adding a BrushShader creates at least 3 new shaders
    • adding something inside of BrushShaderrequires hard-coding the logic to select the variant in a way that works with all brushes.

What I would like from a new shader selection implementation:

  • The list of shaders and theirs feature is specified once in shader_features.rs. In shade.rs, all shaders are built automatically from that list.
  • Shaders are grouped by "kind" for example solid, linear_gradient, composite, etc.
  • Each shader kind can have any combination of shader variants specified in shader_features.rs
  • When selecting a shader we first look up the kind and iterate its sorted list of variants until we find one that contains all of the requested features.
  • The debug overdraw shader is no longer a variant of each brush (we can have a single brush_debug_overdraw).
  • All features in the bitfield directly map to a #define in the shaders

The shader list could look something in the spirit of:

// Use the bitfield instead of strings, the feature strings will be derived from it.
use ShaderFeatures::*;
&[
    (
        Shaderkind::BrushSolid,
        &[
            OPAQUE,
            ALPHA,
            ALPHA | ANTIALIASING],
    ),
    (
        ShaderKind::BrushImage,
        &[
            TEXTURE_2D | OPAQUE,
            TEXTURE_2D | OPAQUE | REPETITIONS,
            TEXTURE_2D | ALPHA,
            TEXTURE_2D | ALPHA | ANTIALIASING | REPETITIONS,
            TEXTURE_EXTERNAL | OPAQUE,
            TEXTURE_EXTERNAL | OPAQUE | REPETITIONS,
            TEXTURE_EXTERNAL | ALPHA,
            TEXTURE_EXTERNAL | ALPHA | ANTIALIASING | REPETITIONS,
            TEXTURE_RECT | OPAQUE,
            TEXTURE_RECT | OPAQUE | REPETITIONS,
            TEXTURE_RECT | ALPHA,
            TEXTURE_RECT | ALPHA | ANTIALIASING | REPETITIONS,
        ],
    ),
   // etc...
];

Or we could do programmatically populate the list as we currently do. In this case I think that having to repeat ourselves a bit would help keep the number of permutations under control.

Adding a new shader variant to a subset of the shaders should only require adding the feature where needed in the shader list, the corresponding code in the shader source and nothing else. The shade.rs there should be no shader specific logic.

Blocks: wr-todos
No longer blocks: gfx-complexity
You need to log in before you can comment on or make changes to this bug.