Auto merge of #12543 - mrobinson:off-by-one-ng, r=pcwalton

Accumulate subpixels through stacking contexts

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Instead of simply rounding layer origins and discarding subpixel
offsets, accumulate them by transforming them into the space of the
next child stacking context. This is an attempt to eliminate subpixel
differences that are caused by different stacking context boundaries in
reference tests.

Currently these accumulated subpixels are only used for text
positioning, but the plan is that they can be used for all drawing in
the future.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12543)
<!-- Reviewable:end -->

5 files changed, 58 insertions, 25 deletions

diff --git a/components/gfx/display_list/mod.rs b/components/gfx/display_list/mod.rs
index 1d67ceddd2f..0073b1b0a69 100644
--- a/components/gfx/display_list/mod.rs
+++ b/components/gfx/display_list/mod.rs
@@ -382,7 +382,11 @@ impl DisplayList {
             current_item_index: start,
             last_item_index: end,
         };
-        self.draw_stacking_context(stacking_context, &mut traversal, paint_context, transform);
+        self.draw_stacking_context(stacking_context,
+                                   &mut traversal,
+                                   paint_context,
+                                   transform,
+                                   &Point2D::zero());
     }
 
     fn draw_stacking_context_contents<'a>(&'a self,
@@ -390,6 +394,7 @@ impl DisplayList {
                                           traversal: &mut DisplayListTraversal<'a>,
                                           paint_context: &mut PaintContext,
                                           transform: &Matrix4D<f32>,
+                                          subpixel_offset: &Point2D<Au>,
                                           tile_rect: Option<Rect<Au>>) {
         for child in stacking_context.children.iter() {
             while let Some(item) = traversal.advance(stacking_context) {
@@ -399,7 +404,11 @@ impl DisplayList {
             }
 
             if child.intersects_rect_in_parent_context(tile_rect) {
-                self.draw_stacking_context(child, traversal, paint_context, &transform);
+                self.draw_stacking_context(child,
+                                           traversal,
+                                           paint_context,
+                                           &transform,
+                                           subpixel_offset);
             } else {
                 traversal.skip_past_stacking_context(child);
             }
@@ -417,12 +426,14 @@ impl DisplayList {
                                  stacking_context: &StackingContext,
                                  traversal: &mut DisplayListTraversal<'a>,
                                  paint_context: &mut PaintContext,
-                                 transform: &Matrix4D<f32>) {
+                                 transform: &Matrix4D<f32>,
+                                 subpixel_offset: &Point2D<Au>) {
         if stacking_context.context_type != StackingContextType::Real {
             self.draw_stacking_context_contents(stacking_context,
                                                 traversal,
                                                 paint_context,
                                                 transform,
+                                                subpixel_offset,
                                                 None);
             return;
         }
@@ -431,18 +442,35 @@ impl DisplayList {
             &stacking_context.filters,
             stacking_context.blend_mode);
 
-        // If a layer is being used, the transform for this layer
-        // will be handled by the compositor.
         let old_transform = paint_context.draw_target.get_transform();
-        let transform = match stacking_context.layer_info {
-            Some(..) => *transform,
+        let pixels_per_px = paint_context.screen_pixels_per_px();
+        let (transform, subpixel_offset) = match stacking_context.layer_info {
+            // If this stacking context starts a layer, the offset and transformation are handled
+            // by layer position within the compositor.
+            Some(..) => (*transform, *subpixel_offset),
             None => {
-                let pixels_per_px = paint_context.screen_pixels_per_px();
-                let origin = &stacking_context.bounds.origin;
-                transform.translate(
-                    origin.x.to_nearest_pixel(pixels_per_px.get()) as AzFloat,
-                    origin.y.to_nearest_pixel(pixels_per_px.get()) as AzFloat,
-                    0.0).mul(&stacking_context.transform)
+                let origin = stacking_context.bounds.origin + *subpixel_offset;
+                let pixel_snapped_origin =
+                    Point2D::new(origin.x.to_nearest_pixel(pixels_per_px.get()),
+                                 origin.y.to_nearest_pixel(pixels_per_px.get()));
+
+                let transform = transform.translate(pixel_snapped_origin.x as AzFloat,
+                                                    pixel_snapped_origin.y as AzFloat,
+                                                    0.0).mul(&stacking_context.transform);
+                let inverse_transform = transform.invert();
+
+                // Here we are trying to accumulate any subpixel distances across transformed
+                // stacking contexts. This allows us transform stacking context with a
+                // pixel-snapped transform, but continue to propagate any subpixels from stacking
+                // context origins to children.
+                let subpixel_offset = Point2D::new(origin.x.to_f32_px() - pixel_snapped_origin.x,
+                                                   origin.y.to_f32_px() - pixel_snapped_origin.y);
+                let subpixel_offset = inverse_transform.transform_point(&subpixel_offset) -
+                                      inverse_transform.transform_point(&Point2D::zero());;
+                let subpixel_offset = Point2D::new(Au::from_f32_px(subpixel_offset.x),
+                                                   Au::from_f32_px(subpixel_offset.y));
+
+                (transform, subpixel_offset)
             }
         };
 
@@ -455,6 +483,7 @@ impl DisplayList {
                 clip_rect: Some(stacking_context.overflow),
                 transient_clip: None,
                 layer_kind: paint_context.layer_kind,
+                subpixel_offset: subpixel_offset,
             };
 
             // Set up our clip rect and transform.
@@ -469,6 +498,7 @@ impl DisplayList {
                 traversal,
                 &mut paint_subcontext,
                 &transform,
+                &subpixel_offset,
                 Some(transformed_tile_rect(paint_context.screen_rect, &transform)));
 
             paint_subcontext.remove_transient_clip_if_applicable();
diff --git a/components/gfx/paint_context.rs b/components/gfx/paint_context.rs
index 2614e77fc82..15c4b169ba4 100644
--- a/components/gfx/paint_context.rs
+++ b/components/gfx/paint_context.rs
@@ -53,6 +53,10 @@ pub struct PaintContext<'a> {
     pub transient_clip: Option<ClippingRegion>,
     /// A temporary hack to disable clipping optimizations on 3d layers.
     pub layer_kind: LayerKind,
+    /// The current subpixel offset, used to make pixel snapping aware of accumulated subpixels
+    /// from the StackingContext.
+    /// TODO: Eventually this should be added to all points handled by the PaintContext.
+    pub subpixel_offset: Point2D<Au>,
 }
 
 #[derive(Copy, Clone)]
@@ -1338,24 +1342,26 @@ impl<'a> PaintContext<'a> {
     pub fn draw_text(&mut self, text: &TextDisplayItem) {
         let draw_target_transform = self.draw_target.get_transform();
 
+        let origin = text.baseline_origin + self.subpixel_offset;
+
         // Optimization: Don’t set a transform matrix for upright text, and pass a start point to
         // `draw_text_into_context`.
         //
         // For sideways text, it’s easier to do the rotation such that its center (the baseline’s
         // start point) is at (0, 0) coordinates.
         let baseline_origin = match text.orientation {
-            Upright => text.baseline_origin,
+            Upright => origin,
             SidewaysLeft => {
-                let x = text.baseline_origin.x.to_f32_px();
-                let y = text.baseline_origin.y.to_f32_px();
+                let x = origin.x.to_f32_px();
+                let y = origin.y.to_f32_px();
                 self.draw_target.set_transform(&draw_target_transform.mul(&Matrix2D::new(0., -1.,
                                                                                          1., 0.,
                                                                                          x, y)));
                 Point2D::zero()
             }
             SidewaysRight => {
-                let x = text.baseline_origin.x.to_f32_px();
-                let y = text.baseline_origin.y.to_f32_px();
+                let x = origin.x.to_f32_px();
+                let y = origin.y.to_f32_px();
                 self.draw_target.set_transform(&draw_target_transform.mul(&Matrix2D::new(0., 1.,
                                                                                          -1., 0.,
                                                                                          x, y)));
@@ -1382,10 +1388,7 @@ impl<'a> PaintContext<'a> {
         // Blur, if necessary.
         self.blur_if_necessary(temporary_draw_target, text.blur_radius);
 
-        // Undo the transform, only when we did one.
-        if text.orientation != Upright {
-            self.draw_target.set_transform(&draw_target_transform)
-        }
+        self.draw_target.set_transform(&draw_target_transform)
     }
 
     /// Draws a linear gradient in the given boundaries from the given start point to the given end
diff --git a/components/gfx/paint_thread.rs b/components/gfx/paint_thread.rs
index b4ad2abd299..1e4d8f77ddc 100644
--- a/components/gfx/paint_thread.rs
+++ b/components/gfx/paint_thread.rs
@@ -690,6 +690,7 @@ impl WorkerThread {
                 clip_rect: None,
                 transient_clip: None,
                 layer_kind: layer_kind,
+                subpixel_offset: Point2D::zero(),
             };
 
             // Apply the translation to paint the tile we want.
diff --git a/tests/wpt/metadata-css/css-transforms-1_dev/html/css-transforms-3d-on-anonymous-block-001.htm.ini b/tests/wpt/metadata-css/css-transforms-1_dev/html/css-transforms-3d-on-anonymous-block-001.htm.ini
index 605b45575ea..09c75600242 100644
--- a/tests/wpt/metadata-css/css-transforms-1_dev/html/css-transforms-3d-on-anonymous-block-001.htm.ini
+++ b/tests/wpt/metadata-css/css-transforms-1_dev/html/css-transforms-3d-on-anonymous-block-001.htm.ini
@@ -1,4 +1,2 @@
 [css-transforms-3d-on-anonymous-block-001.htm]
   type: reftest
-  expected:
-    if os == "linux": FAIL
diff --git a/tests/wpt/metadata-css/css-transforms-1_dev/html/transform-input-019.htm.ini b/tests/wpt/metadata-css/css-transforms-1_dev/html/transform-input-019.htm.ini
index e82b6718013..8e2a7b49f90 100644
--- a/tests/wpt/metadata-css/css-transforms-1_dev/html/transform-input-019.htm.ini
+++ b/tests/wpt/metadata-css/css-transforms-1_dev/html/transform-input-019.htm.ini
@@ -1,3 +1,4 @@
 [transform-input-019.htm]
   type: reftest
-  expected: FAIL
+  expected:
+    if os == "linux": FAIL