Open
Bug 1760285
Opened 3 years ago
Updated 1 year ago
Rewrite the shader selection logic
Categories
(Core :: Graphics: WebRender, task, P4)
Core
Graphics: WebRender
Tracking
()
NEW
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::ANTIALIASINGimplies#define WR_FEATURE_ANTIALIASINGin 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
BrushShadercreates 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.
- adding a
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.
| Reporter | ||
Updated•1 year ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•