Bug 1617524 - Fix crash in get_relative_transform edge case. r=jnicol

Previously, WR would attempt to detect at the start of frame
building if the spatial node of any picture cache contained
a non-axis-aligned transform, and disable picture caching in
that edge case.

However, picture caching can't (currently) be disabled when the
native compositor is active. In this mode, picture caching was
force enabled, causing an assertion failure due to unexpected
coordinate systems when updating pictures.

This patch changes the way the detection of scroll root logic
works such that we don't consider any scroll frame inside a
reference frame to be a valid scroll root for picture caching
purposes. Thus it's not possible to create a picture cache
where the reference spatial node has a non-axis-aligned transform.

Differential Revision: https://phabricator.services.mozilla.com/D75890

gfx/webrender_bindings/src/bindings.rs

@@ -2240,6 +2240,7 @@ pub extern "C" fn wr_dp_clear_save(state: &mut WrState) {
 pub enum WrReferenceFrameKind {
     Transform,
     Perspective,
+    Zoom,
 }
 
 /// IMPORTANT: If you add fields to this struct, you need to also add initializers
@@ -2353,6 +2354,7 @@ pub extern "C" fn wr_dp_push_stacking_context(
         let reference_frame_kind = match params.reference_frame_kind {
             WrReferenceFrameKind::Transform => ReferenceFrameKind::Transform,
             WrReferenceFrameKind::Perspective => ReferenceFrameKind::Perspective { scrolling_relative_to },
+            WrReferenceFrameKind::Zoom => ReferenceFrameKind::Zoom,
         };
         wr_spatial_id = state.frame_builder.dl_builder.push_reference_frame(
             bounds.origin,

gfx/wr/webrender/src/clip.rs

@@ -98,7 +98,7 @@ use api::units::*;
 use api::image_tiling::{self, Repetition};
 use crate::border::{ensure_no_corner_overlap, BorderRadiusAu};
 use crate::box_shadow::{BLUR_SAMPLE_SCALE, BoxShadowClipSource, BoxShadowCacheKey};
-use crate::spatial_tree::{ROOT_SPATIAL_NODE_INDEX, SpatialTree, SpatialNodeIndex};
+use crate::spatial_tree::{ROOT_SPATIAL_NODE_INDEX, SpatialTree, SpatialNodeIndex, CoordinateSystemId};
 use crate::ellipse::Ellipse;
 use crate::gpu_cache::{GpuCache, GpuCacheHandle, ToGpuBlocks};
 use crate::gpu_types::{BoxShadowStretchMode};
@@ -884,8 +884,27 @@ impl ClipChainStack {
     /// stack of clips to be propagated.
     pub fn push_surface(
         &mut self,
-        shared_clips: &[ClipInstance],
+        maybe_shared_clips: &[ClipInstance],
+        spatial_tree: &SpatialTree,
     ) {
+        let mut shared_clips = Vec::new();
+
+        // If there are clips in the shared list for a picture cache, only include
+        // them if they are simple, axis-aligned clips (i.e. in the root coordinate
+        // system). This is necessary since when compositing picture cache tiles
+        // into the parent, we don't support applying a clip mask. This only ever
+        // occurs in wrench tests, not in display lists supplied by Gecko.
+        // TODO(gw): We can remove this when we update the WR API to have better
+        //           knowledge of what coordinate system a clip must be in (by
+        //           knowing if a reference frame exists in the chain between the
+        //           clip's spatial node and the picture cache reference spatial node).
+        for clip in maybe_shared_clips {
+            let spatial_node = &spatial_tree.spatial_nodes[clip.spatial_node_index.0 as usize];
+            if spatial_node.coordinate_system_id == CoordinateSystemId::root() {
+                shared_clips.push(*clip);
+            }
+        }
+
         let level = ClipChainLevel {
             shared_clips: shared_clips.to_vec(),
             first_clip_index: self.clips.len(),

gfx/wr/webrender/src/frame_builder.rs

@@ -6,7 +6,7 @@ use api::{ColorF, DebugFlags, DocumentLayer, FontRenderMode, PremultipliedColorF
 use api::units::*;
 use crate::batch::{BatchBuilder, AlphaBatchBuilder, AlphaBatchContainer};
 use crate::clip::{ClipStore, ClipChainStack, ClipInstance};
-use crate::spatial_tree::{SpatialTree, ROOT_SPATIAL_NODE_INDEX, SpatialNodeIndex, CoordinateSystemId};
+use crate::spatial_tree::{SpatialTree, ROOT_SPATIAL_NODE_INDEX, SpatialNodeIndex};
 use crate::composite::{CompositorKind, CompositeState};
 use crate::debug_render::DebugItem;
 use crate::gpu_cache::{GpuCache, GpuCacheHandle};
@@ -153,10 +153,11 @@ impl<'a> FrameVisibilityState<'a> {
     pub fn push_surface(
         &mut self,
         surface_index: SurfaceIndex,
-        shared_clips: &[ClipInstance]
+        shared_clips: &[ClipInstance],
+        spatial_tree: &SpatialTree,
     ) {
         self.surface_stack.push(surface_index);
-        self.clip_chain_stack.push_surface(shared_clips);
+        self.clip_chain_stack.push_surface(shared_clips, spatial_tree);
     }
 
     pub fn pop_surface(&mut self) {
@@ -566,8 +567,7 @@ impl FrameBuilder {
                 let spatial_node = &scene
                     .spatial_tree
                     .spatial_nodes[spatial_node_index.0 as usize];
-                spatial_node.coordinate_system_id != CoordinateSystemId::root() ||
-                    spatial_node.is_ancestor_or_self_zooming
+                spatial_node.is_ancestor_or_self_zooming
             });
 
         let mut composite_state = CompositeState::new(

gfx/wr/webrender/src/prim_store/mod.rs

@@ -1906,12 +1906,20 @@ impl PrimitiveStore {
 
                     // Push a new surface, supplying the list of clips that should be
                     // ignored, since they are handled by clipping when drawing this surface.
-                    frame_state.push_surface(surface_index, &tile_cache.shared_clips);
+                    frame_state.push_surface(
+                        surface_index,
+                        &tile_cache.shared_clips,
+                        frame_context.spatial_tree,
+                    );
                     frame_state.tile_cache = Some(tile_cache);
                 }
                 _ => {
                     if is_composite {
-                        frame_state.push_surface(surface_index, &[]);
+                        frame_state.push_surface(
+                            surface_index,
+                            &[],
+                            frame_context.spatial_tree,
+                        );
                     }
                 }
             }

gfx/wr/webrender/src/spatial_node.rs

@@ -340,7 +340,7 @@ impl SpatialNode {
                                 .post_translate(-scroll_offset)
                         }
                         ReferenceFrameKind::Perspective { scrolling_relative_to: None } |
-                        ReferenceFrameKind::Transform => source_transform,
+                        ReferenceFrameKind::Transform | ReferenceFrameKind::Zoom => source_transform,
                     };
 
                     let resolved_transform =

gfx/wr/webrender/src/spatial_tree.rs

@@ -631,15 +631,30 @@ impl SpatialTree {
         while node_index != ROOT_SPATIAL_NODE_INDEX {
             let node = &self.spatial_nodes[node_index.0 as usize];
             match node.node_type {
-                SpatialNodeType::ReferenceFrame(..) |
-                SpatialNodeType::StickyFrame(..) => {
-                    // TODO(gw): In future, we may need to consider sticky frames.
+                SpatialNodeType::ReferenceFrame(ref info) => {
+                    match info.kind {
+                        ReferenceFrameKind::Zoom => {
+                            // We can handle scroll nodes that pass through a zoom node
+                        }
+                        ReferenceFrameKind::Transform |
+                        ReferenceFrameKind::Perspective { .. } => {
+                            // When a reference frame is encountered, forget any scroll roots
+                            // we have encountered, as they may end up with a non-axis-aligned transform.
+                            scroll_root = ROOT_SPATIAL_NODE_INDEX;
+                        }
+                    }
                 }
+                SpatialNodeType::StickyFrame(..) => {}
                 SpatialNodeType::ScrollFrame(ref info) => {
-                    // If we found an explicit scroll root, store that
-                    // and keep looking up the tree.
-                    if let ScrollFrameKind::Explicit = info.frame_kind {
-                        scroll_root = node_index;
+                    match info.frame_kind {
+                        ScrollFrameKind::PipelineRoot => {
+                            // Once we encounter a pipeline root, there is no need to look further
+                            break;
+                        }
+                        ScrollFrameKind::Explicit => {
+                            // Store the explicit scroll root, keep looking up the tree
+                            scroll_root = node_index;
+                        }
                     }
                 }
             }

gfx/wr/webrender_api/src/display_item.rs

@@ -711,7 +711,11 @@ pub struct ReferenceFrameDisplayListItem {
 
 #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize, PeekPoke)]
 pub enum ReferenceFrameKind {
+    /// Zoom reference frames must be a scale + translation only
+    Zoom,
+    /// A normal transform matrix, may contain perspective (the CSS transform property)
     Transform,
+    /// A perspective transform, that optionally scrolls relative to a specific scroll node
     Perspective {
         scrolling_relative_to: Option<ExternalScrollId>,
     }

gfx/wr/wrench/reftests/filters/reftest.list

@@ -58,7 +58,7 @@ platform(linux,mac) == draw_calls(8) color_targets(8) alpha_targets(0) svg-filte
 platform(linux,mac) == svg-filter-drop-shadow.yaml svg-filter-drop-shadow.png
 == fuzzy(1,10000) svg-srgb-to-linear.yaml srgb-to-linear-ref.yaml
 platform(linux,mac) == fuzzy(4,28250) svg-filter-drop-shadow-rotate.yaml svg-filter-drop-shadow-rotate-ref.yaml
-platform(linux,mac) == svg-filter-blur-transforms.yaml svg-filter-blur-transforms.png
+platform(linux,mac) fuzzy(3,3184) == svg-filter-blur-transforms.yaml svg-filter-blur-transforms.png
 platform(linux,mac) == svg-filter-drop-shadow-on-viewport-edge.yaml svg-filter-drop-shadow-on-viewport-edge.png
 platform(linux,mac) == svg-filter-drop-shadow-perspective.yaml svg-filter-drop-shadow-perspective.png
 == backdrop-filter-basic.yaml backdrop-filter-basic-ref.yaml

layout/painting/nsDisplayList.cpp

@@ -6348,6 +6348,9 @@ bool nsDisplayOwnLayer::CreateWebRenderCommands(
   if (IsScrollThumbLayer()) {
     params.prim_flags |= wr::PrimitiveFlags::IS_SCROLLBAR_THUMB;
   }
+  if (IsZoomingLayer()) {
+    params.reference_frame_kind = wr::WrReferenceFrameKind::Zoom;
+  }
   StackingContextHelper sc(aSc, GetActiveScrolledRoot(), mFrame, this, aBuilder,
                            params);