1615613 - Unify the text and brush shader infrastructures

Status: RESOLVED WONTFIX

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

Description

[Nicolas Silva [:nical]]

Following up on bug 1594128, we want to be able to batch text and images together to avoid some bad batching situations.

To that end we need the brush shader infrastructure to be able to deocde glyph information, preferrably without adding tons of extra branches. The first step is to make sure that common data is passed the same way in both types of shaders, and in more tricky cases change the way the data is encoded so that things match.

In particular, there is a separation between common brush data and shader specific data. The former is decoded and used early in brush.glsl, while the latter is used in the brush-specific entry points. Examples of shader-specific data include the target of the instance resource address and the content of the primitive user data. Unfortunately the text shader uses some of that information to construct the glyph segments rects (which have to be decoded before the shader-specifc code runs) so we can either:

• Change the way glyph rects are encoded, for example by using the same format as other brushes (likely at the cost of more memory per glyph segment).
• have text-specific code in the common brush infrastructure (at the cost of messy code)
• wrap the whole brush infrastructure into a branch and treat text seprately altogether (will likely produce very bad and/or slow to compile code or the image+text shader).

Comment 1

[Nicolas Silva [:nical]]

Attachment: Bug 1615613 - Use write_vertex in the text run shader. r=gw

Baby step towards making text and brush shaders more similar.

Comment 2

[Nicolas Silva [:nical]]

Attachment: Bug 1615613 - Move text snap bias in its own function. r=gw

Comment 3

[Nicolas Silva [:nical]]

Attachment: Bug 1615613 - Pass the shader color mode via instance vertex attributes. r=gw

The text shader currently needs it per-instance. This unfortunately requires quite a bit of plumbing for something that isn't used very often outside of the text shader. Hopefully we'll be able to tiddy this up when the shading infrastructure is more unified.

the color mode is now packed into 8 bits of the w filed of the instance data, along with the brush kind and shader resource address. This frees up the low bits in the image shader data as a result of which the alpha type is not shifted anymore. For shaders that don't use the color mode, ShaderColorMode::Image is used by default.

Depends on D62926

Comment 4

[Nicolas Silva [:nical]]

Attachment: Bug 1615613 - Wrap image brush user data in a struct. r=gw

A quality-of-life improvement that will make it easier to change the encoding of the user data without having to repeat the correct casting, bit shifting and masking in many places. It also makes it harder to encode the data incorrectly by mistake or forget information.

Depends on D62927

Comment 5

[Nicolas Silva [:nical]]

Attachment: Bug 1615613 - Make the text fragment shader compatible with the brush infrastructure. r=gw

Comment 6

[Nicolas Silva [:nical]]

Attachment: Bug 1615613 - Integrate the text shader in the brush infrastructure. r=gw

The first step is to isolate the parts that are common to both shaders: the code that fetches various piece of data. Hopefully we can devise a way to share more code in followups.

Depends on D63093

Comment 7

[Nicolas Silva [:nical]]

Attachment: Bug 1615613 - Encode glyph instance attributes in gpu_types.rs. r=gw

Continuing on the trend of having all of the gpu data encoding in gpu_types.rs so that it is easy to find and to avoid repeating it in batch.rs.

Depends on D62928

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1c689f1f572
Use write_vertex in the text run shader. r=gw
https://hg.mozilla.org/integration/autoland/rev/fd0e8d05b87e
Move text snap bias in its own function. r=gw
https://hg.mozilla.org/integration/autoland/rev/bb2addbc244c
Wrap image brush user data in a struct. r=gw
https://hg.mozilla.org/integration/autoland/rev/a8fe6bdade92
Encode glyph instance attributes in gpu_types.rs. r=gw

Comment 9

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/d1c689f1f572
https://hg.mozilla.org/mozilla-central/rev/fd0e8d05b87e
https://hg.mozilla.org/mozilla-central/rev/bb2addbc244c
https://hg.mozilla.org/mozilla-central/rev/a8fe6bdade92

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/caa7afb0c06e
Make the text fragment shader compatible with the brush infrastructure. r=gw

Comment 11

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/caa7afb0c06e

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26255dd68307
Integrate the text shader in the brush infrastructure. r=gw

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/26255dd68307

Comment 14

[Nicolas Silva [:nical]]

Marking as stalled because the last steps are diifcult to pull off and we have lower hanging fruits to pick for now.

Comment 15

[Nicolas Silva [:nical]]

Closing for now, we'll reevaluate if/when needed.

Comment 16

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

Since the bug is closed, the stalled keyword is now meaningless.
For more information, please visit BugBot documentation.