Open Bug 1615613 Opened 2 months ago Updated 7 days ago

Unify the text and brush shader infrastructures

Categories

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

task

Tracking

()

ASSIGNED

People

(Reporter: nical, Assigned: nical)

References

(Blocks 1 open bug)

Details

(Keywords: leave-open)

Attachments

(6 files, 1 obsolete file)

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).

Baby step towards making text and brush shaders more similar.

Assignee: nobody → nical.bugzilla
Status: NEW → ASSIGNED

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

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

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

Keywords: leave-open

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

Attachment #9127120 - Attachment description: Bug 1615613 - Begin integrating the text shader in the brush infrastructure. r=gw → Bug 1615613 - Integrate the text shader in the brush infrastructure. r=gw
Pushed by nsilva@mozilla.com:
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
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/caa7afb0c06e
Make the text fragment shader compatible with the brush infrastructure. r=gw
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/26255dd68307
Integrate the text shader in the brush infrastructure. r=gw
Regressions: 1623716
Attachment #9126748 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.