Bug 1546818 - WR text local transform fix r=gw
authorDzmitry Malyshau <dmalyshau@mozilla.com>
Wed, 03 Jul 2019 18:59:42 +0000
changeset 544000 0bee4245b65a13592d6ec4976ad5562056dbd982
parent 543999 16f07487dbb277313057c0abbdb04c7f6dd0cb91
child 544001 8a08f6450ed10f1bfdefb1f4c0af0b71da7bd55f
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgw
bugs1546818
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1546818 - WR text local transform fix r=gw Change the glyph transform computation to be relative to the surface node. Differential Revision: https://phabricator.services.mozilla.com/D36603
gfx/wr/webrender/res/ps_text_run.glsl
gfx/wr/webrender/res/rect.glsl
gfx/wr/webrender/src/picture.rs
gfx/wr/webrender/src/prim_store/mod.rs
gfx/wr/webrender/src/prim_store/text_run.rs
gfx/wr/wrench/reftests/text/intermediate-transform.yaml
gfx/wr/wrench/reftests/text/reftest.list
layout/reftests/transform-3d/reftest.list
--- a/gfx/wr/webrender/res/ps_text_run.glsl
+++ b/gfx/wr/webrender/res/ps_text_run.glsl
@@ -13,16 +13,29 @@ flat varying vec2 vMaskSwizzle;
 varying vec4 vUvClip;
 #endif
 
 #ifdef WR_VERTEX_SHADER
 
 #define VECS_PER_TEXT_RUN           2
 #define GLYPHS_PER_GPU_BLOCK        2U
 
+#ifdef WR_FEATURE_GLYPH_TRANSFORM
+RectWithSize transform_rect(RectWithSize rect, mat2 transform) {
+    vec2 center = transform * (rect.p0 + rect.size * 0.5);
+    vec2 radius = mat2(abs(transform[0]), abs(transform[1])) * (rect.size * 0.5);
+    return RectWithSize(center - radius, radius * 2.0);
+}
+
+bool rect_inside_rect(RectWithSize little, RectWithSize big) {
+    return all(lessThanEqual(vec4(big.p0, little.p0 + little.size),
+                             vec4(little.p0, big.p0 + big.size)));
+}
+#endif //WR_FEATURE_GLYPH_TRANSFORM
+
 struct Glyph {
     vec2 offset;
 };
 
 Glyph fetch_glyph(int specific_prim_address,
                   int glyph_index) {
     // Two glyphs are packed in each texel in the GPU cache.
     int glyph_address = specific_prim_address +
@@ -66,49 +79,50 @@ VertexInfo write_text_vertex(RectWithSiz
                              Transform transform,
                              PictureTask task,
                              vec2 text_offset,
                              vec2 glyph_offset,
                              RectWithSize glyph_rect,
                              vec2 snap_bias) {
     // The offset to snap the glyph rect to a device pixel
     vec2 snap_offset = vec2(0.0);
-    mat2 local_transform = mat2(1.0);
+    // Transform from glyph space to local space
+    mat2 glyph_transform_inv = mat2(1.0);
 
 #ifdef WR_FEATURE_GLYPH_TRANSFORM
     bool remove_subpx_offset = true;
 #else
     bool remove_subpx_offset = transform.is_axis_aligned;
 #endif
     // Compute the snapping offset only if the scroll node transform is axis-aligned.
     if (remove_subpx_offset) {
         // Be careful to only snap with the transform when in screen raster space.
         switch (raster_space) {
             case RASTER_SCREEN: {
-                // Transform from local space to device space.
+                // Transform from local space to glyph space.
                 float device_scale = task.device_pixel_scale / transform.m[3].w;
-                mat2 device_transform = mat2(transform.m) * device_scale;
+                mat2 glyph_transform = mat2(transform.m) * device_scale;
 
                 // Ensure the transformed text offset does not contain a subpixel translation
                 // such that glyph snapping is stable for equivalent glyph subpixel positions.
-                vec2 device_text_pos = device_transform * text_offset + transform.m[3].xy * device_scale;
+                vec2 device_text_pos = glyph_transform * text_offset + transform.m[3].xy * device_scale;
                 snap_offset = floor(device_text_pos + 0.5) - device_text_pos;
 
                 // Snap the glyph offset to a device pixel, using an appropriate bias depending
                 // on whether subpixel positioning is required.
-                vec2 device_glyph_offset = device_transform * glyph_offset;
+                vec2 device_glyph_offset = glyph_transform * glyph_offset;
                 snap_offset += floor(device_glyph_offset + snap_bias) - device_glyph_offset;
 
-                // Transform from device space back to local space.
-                local_transform = inverse(device_transform);
+                // Transform from glyph space back to local space.
+                glyph_transform_inv = inverse(glyph_transform);
 
 #ifndef WR_FEATURE_GLYPH_TRANSFORM
                 // If not using transformed subpixels, the glyph rect is actually in local space.
                 // So convert the snap offset back to local space.
-                snap_offset = local_transform * snap_offset;
+                snap_offset = glyph_transform_inv * snap_offset;
 #endif
                 break;
             }
             default: {
                 // Otherwise, when in local raster space, the transform may be animated, so avoid
                 // snapping with the transform to avoid oscillation.
                 snap_offset = floor(text_offset + 0.5) - text_offset;
                 snap_offset += floor(glyph_offset + snap_bias) - glyph_offset;
@@ -117,26 +131,26 @@ VertexInfo write_text_vertex(RectWithSiz
         }
     }
 
     // Actually translate the glyph rect to a device pixel using the snap offset.
     glyph_rect.p0 += snap_offset;
 
 #ifdef WR_FEATURE_GLYPH_TRANSFORM
     // The glyph rect is in device space, so transform it back to local space.
-    RectWithSize local_rect = transform_rect(glyph_rect, local_transform);
+    RectWithSize local_rect = transform_rect(glyph_rect, glyph_transform_inv);
 
     // Select the corner of the glyph's local space rect that we are processing.
     vec2 local_pos = local_rect.p0 + local_rect.size * aPosition.xy;
 
     // If the glyph's local rect would fit inside the local clip rect, then select a corner from
     // the device space glyph rect to reduce overdraw of clipped pixels in the fragment shader.
     // Otherwise, fall back to clamping the glyph's local rect to the local clip rect.
     if (rect_inside_rect(local_rect, local_clip_rect)) {
-        local_pos = local_transform * (glyph_rect.p0 + glyph_rect.size * aPosition.xy);
+        local_pos = glyph_transform_inv * (glyph_rect.p0 + glyph_rect.size * aPosition.xy);
     }
 #else
     // Select the corner of the glyph rect that we are processing.
     vec2 local_pos = glyph_rect.p0 + glyph_rect.size * aPosition.xy;
 #endif
 
     // Clamp to the local clip rect.
     local_pos = clamp_rect(local_pos, local_clip_rect);
--- a/gfx/wr/webrender/res/rect.glsl
+++ b/gfx/wr/webrender/res/rect.glsl
@@ -23,31 +23,20 @@ RectWithEndpoint to_rect_with_endpoint(R
 RectWithSize to_rect_with_size(RectWithEndpoint rect) {
     RectWithSize result;
     result.p0 = rect.p0;
     result.size = rect.p1 - rect.p0;
 
     return result;
 }
 
-RectWithSize transform_rect(RectWithSize rect, mat2 transform) {
-    vec2 center = transform * (rect.p0 + rect.size * 0.5);
-    vec2 radius = mat2(abs(transform[0]), abs(transform[1])) * (rect.size * 0.5);
-    return RectWithSize(center - radius, radius * 2.0);
-}
-
 RectWithSize intersect_rects(RectWithSize a, RectWithSize b) {
     RectWithSize result;
     result.p0 = max(a.p0, b.p0);
     result.size = min(a.p0 + a.size, b.p0 + b.size) - result.p0;
 
     return result;
 }
 
-bool rect_inside_rect(RectWithSize little, RectWithSize big) {
-    return all(lessThanEqual(vec4(big.p0, little.p0 + little.size),
-                             vec4(little.p0, big.p0 + big.size)));
-}
-
 float point_inside_rect(vec2 p, vec2 p0, vec2 p1) {
     vec2 s = step(p0, p) - step(p1, p);
     return s.x * s.y;
 }
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -1601,17 +1601,17 @@ pub enum Picture3DContext<C> {
 pub struct OrderedPictureChild {
     pub anchor: usize,
     pub spatial_node_index: SpatialNodeIndex,
     pub gpu_address: GpuCacheAddress,
 }
 
 /// Defines the grouping key for a cluster of primitives in a picture.
 /// In future this will also contain spatial grouping details.
-#[derive(Hash, Eq, PartialEq, Copy, Clone)]
+#[derive(Debug, Hash, Eq, PartialEq, Copy, Clone)]
 struct PrimitiveClusterKey {
     /// Grouping primitives by spatial node ensures that we can calculate a local
     /// bounding volume for the cluster, and then transform that by the spatial
     /// node transform once to get an updated bounding volume for the entire cluster.
     spatial_node_index: SpatialNodeIndex,
     /// We want to separate clusters that have different backface visibility properties
     /// so that we can accept / reject an entire cluster at once if the backface is not
     /// visible.
@@ -1784,16 +1784,20 @@ impl PrimitiveList {
                     clusters.push(PrimitiveCluster::new(
                         prim_instance.spatial_node_index,
                         is_backface_visible,
                     ));
                     index
                 }
             );
 
+            if prim_instance.is_chased() {
+                println!("\tcluster {} with {:?}", cluster_index, key);
+            }
+
             // Pictures don't have a known static local bounding rect (they are
             // calculated during the picture traversal dynamically). If not
             // a picture, include a minimal bounding rect in the cluster bounds.
             let cluster = &mut clusters[cluster_index];
             if !is_pic {
                 let prim_rect = LayoutRect::new(
                     prim_instance.prim_origin,
                     prim_size,
--- a/gfx/wr/webrender/src/prim_store/mod.rs
+++ b/gfx/wr/webrender/src/prim_store/mod.rs
@@ -2089,16 +2089,20 @@ impl PrimitiveStore {
                     }
                 };
 
                 if prim_instance.is_chased() {
                     println!("\teffective clip chain from {:?} {}",
                         clip_chain.clips_range,
                         if apply_local_clip_rect { "(applied)" } else { "" },
                     );
+                    println!("\tpicture rect {:?} @{:?}",
+                        clip_chain.pic_clip_rect,
+                        clip_chain.pic_spatial_node_index,
+                    );
                 }
 
                 // Check if the clip bounding rect (in pic space) is visible on screen
                 // This includes both the prim bounding rect + local prim clip rect!
                 let world_rect = match map_surface_to_world.map(&clip_chain.pic_clip_rect) {
                     Some(world_rect) => world_rect,
                     None => {
                         continue;
@@ -2198,16 +2202,19 @@ impl PrimitiveStore {
                     };
                     if debug_color.a != 0.0 {
                         let debug_rect = clipped_world_rect * frame_context.global_device_pixel_scale;
                         frame_state.scratch.push_debug_rect(debug_rect, debug_color);
                     }
                 }
 
                 let vis_index = PrimitiveVisibilityIndex(frame_state.scratch.prim_info.len() as u32);
+                if prim_instance.is_chased() {
+                    println!("\tvisible {:?} with {:?}", vis_index, combined_local_clip_rect);
+                }
 
                 frame_state.scratch.prim_info.push(
                     PrimitiveVisibility {
                         clipped_world_rect,
                         clip_chain,
                         clip_task_index: ClipTaskIndex::INVALID,
                         combined_local_clip_rect,
                         snap_offsets,
@@ -2823,32 +2830,38 @@ impl PrimitiveStore {
                 }
             }
             PrimitiveInstanceKind::TextRun { run_index, data_handle, .. } => {
                 let prim_data = &mut data_stores.text_run[*data_handle];
                 let run = &mut self.text_runs[*run_index];
 
                 prim_data.common.may_need_repetition = false;
 
-                // The transform only makes sense for screen space rasterization
-                let relative_transform = frame_context
-                    .clip_scroll_tree
-                    .get_world_transform(prim_instance.spatial_node_index)
-                    .into_transform();
+                // The glyph transform has to match `glyph_transform` in "ps_text_run" shader
+                let transform = frame_context.clip_scroll_tree
+                    .get_world_transform(pic_context.surface_spatial_node_index)
+                    .inverse()
+                    .unwrap()
+                    .into_fast_transform()
+                    .pre_mul(
+                        &frame_context.clip_scroll_tree
+                            .get_world_transform(prim_instance.spatial_node_index)
+                            .into_fast_transform()
+                    );;
                 let prim_offset = prim_instance.prim_origin.to_vector() - run.reference_frame_relative_offset;
 
                 let pic = &self.pictures[pic_context.pic_index.0];
                 let raster_space = pic.get_raster_space(frame_context.clip_scroll_tree);
                 let surface = &frame_state.surfaces[pic_context.surface_index.0];
 
                 run.request_resources(
                     prim_offset,
                     &prim_data.font,
                     &prim_data.glyphs,
-                    &relative_transform,
+                    &transform.to_transform().with_destination::<_>(),
                     surface,
                     raster_space,
                     pic_context.subpixel_mode,
                     frame_state.resource_cache,
                     frame_state.gpu_cache,
                     frame_state.render_tasks,
                     scratch,
                 );
--- a/gfx/wr/webrender/src/prim_store/text_run.rs
+++ b/gfx/wr/webrender/src/prim_store/text_run.rs
@@ -98,16 +98,17 @@ impl TextRunTemplate {
         self.write_prim_gpu_blocks(frame_state);
         self.opacity = PrimitiveOpacity::translucent();
     }
 
     fn write_prim_gpu_blocks(
         &mut self,
         frame_state: &mut FrameBuildingState,
     ) {
+        // corresponds to `fetch_glyph` in the shaders
         if let Some(mut request) = frame_state.gpu_cache.request(&mut self.common.gpu_cache_handle) {
             request.push(ColorF::from(self.font.color).premultiplied());
             // this is the only case where we need to provide plain color to GPU
             let bg_color = ColorF::from(self.font.bg_color);
             request.push([bg_color.r, bg_color.g, bg_color.b, 1.0]);
 
             let mut gpu_block = [0.0; 4];
             for (i, src) in self.glyphs.iter().enumerate() {
@@ -232,26 +233,23 @@ impl TextRunPrimitive {
         //           will no longer be required.
         let raster_scale = raster_space.local_scale().unwrap_or(1.0).max(0.001);
 
         // Get the current font size in device pixels
         let device_font_size = specified_font.size.scale_by(device_pixel_scale.0 * raster_scale);
 
         // Determine if rasterizing glyphs in local or screen space.
         // Only support transforms that can be coerced to simple 2D transforms.
-        let transform_glyphs = if transform.has_perspective_component() ||
-           !transform.has_2d_inverse() ||
-           // Font sizes larger than the limit need to be scaled, thus can't use subpixels.
-           transform.exceeds_2d_scale(FONT_SIZE_LIMIT / device_font_size.to_f64_px()) ||
-           // Otherwise, ensure the font is rasterized in screen-space.
-           raster_space != RasterSpace::Screen {
-            false
-        } else {
-            true
-        };
+        let transform_glyphs =
+            !transform.has_perspective_component() &&
+            transform.has_2d_inverse() &&
+            // Font sizes larger than the limit need to be scaled, thus can't use subpixels.
+            !transform.exceeds_2d_scale(FONT_SIZE_LIMIT / device_font_size.to_f64_px()) &&
+            // Otherwise, ensure the font is rasterized in screen-space.
+            raster_space == RasterSpace::Screen;
 
         // Get the font transform matrix (skew / scale) from the complete transform.
         let font_transform = if transform_glyphs {
             FontTransform::from(transform)
         } else {
             FontTransform::identity()
         };
 
@@ -271,17 +269,18 @@ impl TextRunPrimitive {
             ..specified_font.clone()
         };
 
         // If subpixel AA is disabled due to the backing surface the glyphs
         // are being drawn onto, disable it (unless we are using the
         // specifial subpixel mode that estimates background color).
         if (subpixel_mode == SubpixelMode::Deny && self.used_font.bg_color.a == 0) ||
             // If using local space glyphs, we don't want subpixel AA.
-            !transform_glyphs {
+            !transform_glyphs
+        {
             self.used_font.disable_subpixel_aa();
         }
 
         cache_dirty
     }
 
     pub fn request_resources(
         &mut self,
new file mode 100644
--- /dev/null
+++ b/gfx/wr/wrench/reftests/text/intermediate-transform.yaml
@@ -0,0 +1,54 @@
+# This test case makes the text flipped relative to the surface it renders to, but not to the world.
+---
+root:
+  items:
+    -
+      type: "stacking-context"
+      items:
+        -
+          type: "reference-frame"
+          transform: [0.7753850221633911, 0, 0, 0, 0, 0.7753850221633911, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1]
+          items:
+            -
+              type: "stacking-context"
+              items:
+                -
+                  type: "stacking-context"
+                  transform-style: "preserve-3d"
+                  origin: [237, 246]
+                  items:
+                    -
+                      type: "reference-frame"
+                      transform-style: "preserve-3d"
+                      transform: [1, 0, 0, 0, 0, 1, 0, 0, -0.09215625375509262, -0.05100416764616966, 1, -0.0001250000059371814, 0, -3, 0, 1]
+                      items:
+                        -
+                          type: "stacking-context"
+                          transform-style: "preserve-3d"
+                          items:
+                            -
+                              type: "reference-frame"
+                              transform: [-1, 0, 0, 0, 0, -0.9659258127212524, 0.258819043636322, 0, 0, 0.258819043636322, 0.9659258127212524, 0, 1474.5, 802.0977172851563, -105.5981674194336, 1]
+                              items:
+                                -
+                                  type: "stacking-context"
+                                  items:
+                                    -
+                                      rect: [0, 0, 1475, 408]
+                                      color: red
+                                    -
+                                      type: "reference-frame"
+                                      transform: [-1, 0, 0, 0, 0, -1, 0, 0, 0, 0, 1, 0, 1474.5, 408, 0, 1]
+                                      items:
+                                        -
+                                          type: "stacking-context"
+                                          items:
+                                            -
+                                              glyphs: [55,43,40,3,44,54,36,37,40,47,47,36]
+                                              offsets: [511, 290, 551.2000122070313, 290, 597.066650390625, 290, 637.2666625976563, 290, 656.1333618164063, 290, 682.0999755859375, 290, 716.6333618164063, 290, 759.6666870117188, 290, 799.8666381835938, 290, 840.066650390625, 290, 880.2666625976563, 290, 920.4666748046875, 290]
+                                              size: 22
+                                              color: black
+                                              font: "../text/VeraBd.ttf"
+                                              bounds: [509, 250, 457, 54]
+                                              clip-rect: [508, 249, 459, 56]
+
--- a/gfx/wr/wrench/reftests/text/reftest.list
+++ b/gfx/wr/wrench/reftests/text/reftest.list
@@ -66,8 +66,9 @@ skip_on(android,debug,emulator) skip_on(
 skip_on(android,device) == bg-color.yaml bg-color-ref.yaml  # Fails on Pixel2
 != large-glyphs.yaml blank.yaml
 == snap-text-offset.yaml snap-text-offset-ref.yaml
 == shadow-border.yaml shadow-solid-ref.yaml
 skip_on(android,emulator) == shadow-image.yaml shadow-solid-ref.yaml  # Fails on Android emulator
 skip_on(android,emulator) options(disable-aa) == snap-clip.yaml snap-clip-ref.yaml  # Fails on Android emulator
 platform(linux) == perspective-clip.yaml perspective-clip.png
 fuzzy(1,6) options(disable-subpixel) == raster-space-snap.yaml raster-space-snap-ref.yaml
+# == intermediate-transform.yaml intermediate-transform-ref.yaml # fails because of AA inavailable with an intermediate surface
--- a/layout/reftests/transform-3d/reftest.list
+++ b/layout/reftests/transform-3d/reftest.list
@@ -6,17 +6,17 @@ fuzzy-if(webrender,0-1,0-6) == rotatey-1
 # Check that the perspectve() transform function results in some visual changes
 != rotatex-perspective-1a.html rotatex-1-ref.html
 # Check that -moz-perspective results in visual changes to child transformed elements
 != rotatex-perspective-1b.html rotatex-1-ref.html
 # -moz-perspective should only apply to child elements
 == rotatex-perspective-1c.html rotatex-1-ref.html
 == rotatex-perspective-3a.html rotatex-perspective-3-ref.html
 == scalez-1a.html scalez-1-ref.html
-fuzzy-if(gtkWidget||winWidget,0-8,0-376) fuzzy-if(Android,0-8,0-441) fuzzy-if(cocoaWidget,0-17,0-4) fuzzy-if(skiaContent,0-16,0-286) fuzzy-if(webrender&&winWidget,92-92,240-291) == preserve3d-1a.html preserve3d-1-ref.html
+fuzzy-if(gtkWidget||winWidget,0-8,0-376) fuzzy-if(Android,0-8,0-441) fuzzy-if(cocoaWidget,0-17,0-4) fuzzy-if(skiaContent,0-16,0-286) fuzzy-if(webrender&&cocoaWidget,0-200,0-310) fuzzy-if(webrender&&winWidget,0-175,0-250) == preserve3d-1a.html preserve3d-1-ref.html
 == preserve3d-1b.html about:blank
 == preserve3d-clipped.html about:blank
 == preserve3d-2a.html preserve3d-2-ref.html
 == preserve3d-2b.html preserve3d-2-ref.html
 == preserve3d-2c.html preserve3d-2-ref.html
 == preserve3d-2d.html preserve3d-2-ref.html
 == preserve3d-3a.html preserve3d-3-ref.html
 == preserve3d-4a.html about:blank