Refactor how text shadows and elements are handled. #1498

glennw · 2017-07-19T07:20:18Z

While working on adding line decorations, I kept running into
issues and extra complexity. Instead, I've refactored how
text shadow contexts are handled. The implementation is a bit
complex in parts, but conceptually this fits in much better with
how the frame builder, prim store and GPU cache work.

Now, text-shadow primitives add text runs and decorations as
sub-primitives, using an index buffer of primitives. This means:

The code to manage the text-element stack and maintain correct
paint order is much simpler.
The GPU cache layouts are much simpler. Text runs and decorations
within a text-shadow element have the same layout as normal
primitives (no weird offsets and packed layouts).
Text primitives that have both visual and shadows (which is the
common case) share the same GPU primitive - so they only get
built and uploaded once. This is quite a large performance win
on pages with a lot of shadows.
The code to truncate glyph positions is moved from the CPU
to the vertex shader, which is a significant performance win.
The paint order of shadows is reversed from the previous
implementation, back to what it was originally.

This change is

glennw · 2017-07-19T07:20:23Z

r? @kvark

Sorry for another large patch on this stuff!

kvark

Looks like a great cleanup!

kvark · 2017-07-19T20:35:08Z

webrender/res/cs_text_run.vs.glsl

@@ -22,7 +33,7 @@ void main(void) {
    vec2 size = res.uv_rect.zw - res.uv_rect.xy;
    vec2 local_pos = glyph.offset + vec2(res.offset.x, -res.offset.y) / uDevicePixelRatio;
    vec2 origin = prim.task.render_target_origin +
-                  uDevicePixelRatio * (text.offset + local_pos);
+                  uDevicePixelRatio * (local_pos + shadow.offset - shadow_geom.local_rect.p0);


does everything that goes through cs_text_run has to have a shadow?

kvark · 2017-07-19T20:35:08Z

webrender/src/frame_builder.rs

-                                                     local_clip,
-                                                     clip_and_scroll);
-        } else if color.a > 0.0 {
+        let mut shadows = Vec::new();


let's rename it to something like trivial_shadow?

kvark · 2017-07-19T20:35:08Z

webrender/src/frame_builder.rs

+        for shadow_prim_index in &self.shadow_prim_stack {
+            let shadow_metadata = &self.prim_store.cpu_metadata[shadow_prim_index.0];
+            let shadow_prim = &self.prim_store.cpu_text_shadows[shadow_metadata.cpu_prim_index.0];
+            if shadow_prim.shadow.blur_radius == 0.0 {


somewhat unfortunate that we already have the same loop at the start of the function, maybe we could combine them?

Unfortunately, we need to do part of it before adding the prim and part afterwards.

kvark · 2017-07-19T20:35:08Z

webrender/src/prim_store.rs

+        //           text shadow primitives. Consider restructuring this code to
+        //           avoid borrow checker issues.
+        match prim_kind {
+            PrimitiveKind::TextShadow => {


nit: looks like a case for if let

kvark · 2017-07-19T20:35:35Z

r=me whether or not the notes are addressed


        Refactor how text shadows and elements are handled.

While working on adding line decorations, I kept running into issues and extra complexity. Instead, I've refactored how text shadow contexts are handled. The implementation is a bit complex in parts, but conceptually this fits in much better with how the frame builder, prim store and GPU cache work. Now, text-shadow primitives add text runs and decorations as sub-primitives, using an index buffer of primitives. This means: * The code to manage the text-element stack and maintain correct paint order is much simpler. * The GPU cache layouts are much simpler. Text runs and decorations within a text-shadow element have the same layout as normal primitives (no weird offsets and packed layouts). * Text primitives that have both visual and shadows (which is the common case) share the same GPU primitive - so they only get built and uploaded once. This is quite a large performance win on pages with a lot of shadows. * The code to truncate glyph positions is moved from the CPU to the vertex shader, which is a significant performance win. * The paint order of shadows is reversed from the previous implementation, back to what it was originally.

glennw · 2017-07-19T21:58:13Z

@kvark Thanks, issued addressed or explained above.

@bors-servo r=kvark

bors-servo · 2017-07-19T21:58:14Z

📌 Commit ef21f66 has been approved by kvark

bors-servo · 2017-07-19T22:04:39Z

⌛ Testing commit ef21f66 with merge b83c200...


        Auto merge of #1498 - glennw:better-text-shadows-3, r=kvark

Refactor how text shadows and elements are handled. While working on adding line decorations, I kept running into issues and extra complexity. Instead, I've refactored how text shadow contexts are handled. The implementation is a bit complex in parts, but conceptually this fits in much better with how the frame builder, prim store and GPU cache work. Now, text-shadow primitives add text runs and decorations as sub-primitives, using an index buffer of primitives. This means: * The code to manage the text-element stack and maintain correct paint order is much simpler. * The GPU cache layouts are much simpler. Text runs and decorations within a text-shadow element have the same layout as normal primitives (no weird offsets and packed layouts). * Text primitives that have both visual and shadows (which is the common case) share the same GPU primitive - so they only get built and uploaded once. This is quite a large performance win on pages with a lot of shadows. * The code to truncate glyph positions is moved from the CPU to the vertex shader, which is a significant performance win. * The paint order of shadows is reversed from the previous implementation, back to what it was originally.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1498)

bors-servo · 2017-07-19T23:45:55Z

☀️ Test successful - status-travis
Approved by: kvark
Pushing b83c200 to master...

kvark approved these changes Jul 19, 2017

View changes

glennw force-pushed the glennw:better-text-shadows-3 branch from 017ece1 to ef21f66 Jul 19, 2017

glennw mentioned this pull request Jul 19, 2017

Make ClipScrollGroup per ClipScrollInfo #1500

Merged

bors-servo merged commit ef21f66 into servo:master Jul 19, 2017
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

This was referenced Jul 19, 2017

[DO NOT MERGE] WIP implementation of line decorations. #1494

Closed

Add an experimental geometry api #1461

Closed

servo / webrender

Refactor how text shadows and elements are handled. #1498

Refactor how text shadows and elements are handled. #1498

glennw commented Jul 19, 2017 •

edited by larsbergstrom

glennw commented Jul 19, 2017

kvark left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kvark commented Jul 19, 2017

glennw commented Jul 19, 2017

bors-servo commented Jul 19, 2017

bors-servo commented Jul 19, 2017

bors-servo commented Jul 19, 2017

servo / webrender

Join GitHub today

Refactor how text shadows and elements are handled. #1498

Refactor how text shadows and elements are handled. #1498

Conversation

glennw commented Jul 19, 2017 • edited by larsbergstrom

glennw commented Jul 19, 2017

kvark left a comment

This comment has been minimized.

kvark Jul 19, 2017

This comment has been minimized.

glennw Jul 19, 2017

This comment has been minimized.

kvark Jul 19, 2017

This comment has been minimized.

kvark Jul 19, 2017

This comment has been minimized.

glennw Jul 19, 2017

This comment has been minimized.

kvark Jul 19, 2017

kvark commented Jul 19, 2017

glennw commented Jul 19, 2017

bors-servo commented Jul 19, 2017

bors-servo commented Jul 19, 2017

bors-servo commented Jul 19, 2017

glennw commented Jul 19, 2017 •

edited by larsbergstrom