Backed out 4 changesets (bug 1874199) for causing WR bustages CLOSED TREE

Backed out changeset 0367fe61d2c9 (bug 1874199) Backed out changeset 00d4835ec154 (bug 1874199) Backed out changeset 4c6a338c5c30 (bug 1874199) Backed out changeset 9756d029484d (bug 1874199)
2024-05-02 04:30:55 +03:00 · 2024-05-02 04:30:55 +03:00 · 06d741f4fa
--- a/gfx/webrender_bindings/WebRenderAPI.cpp
+++ b/gfx/webrender_bindings/WebRenderAPI.cpp
@ -1261,12 +1261,11 @@ wr::WrSpatialId DisplayListBuilder::DefineStickyFrame(
    const float* aRightMargin, const float* aBottomMargin,
    const float* aLeftMargin, const StickyOffsetBounds& aVerticalBounds,
    const StickyOffsetBounds& aHorizontalBounds,
-    const wr::LayoutVector2D& aAppliedOffset, wr::SpatialTreeItemKey aKey,
-    const WrAnimationProperty* aAnimation) {
+    const wr::LayoutVector2D& aAppliedOffset, wr::SpatialTreeItemKey aKey) {
  auto spatialId = wr_dp_define_sticky_frame(
      mWrState, mCurrentSpaceAndClipChain.space, aContentRect, aTopMargin,
      aRightMargin, aBottomMargin, aLeftMargin, aVerticalBounds,
-      aHorizontalBounds, aAppliedOffset, aKey, aAnimation);
+      aHorizontalBounds, aAppliedOffset, aKey);

  WRDL_LOG("DefineSticky id=%zu c=%s t=%s r=%s b=%s l=%s v=%s h=%s a=%s\n",
           mWrState, spatialId.id, ToString(aContentRect).c_str(),
--- a/gfx/webrender_bindings/WebRenderAPI.h
+++ b/gfx/webrender_bindings/WebRenderAPI.h
@ -600,8 +600,7 @@ class DisplayListBuilder final {
      const float* aRightMargin, const float* aBottomMargin,
      const float* aLeftMargin, const StickyOffsetBounds& aVerticalBounds,
      const StickyOffsetBounds& aHorizontalBounds,
-      const wr::LayoutVector2D& aAppliedOffset, wr::SpatialTreeItemKey aKey,
-      const WrAnimationProperty* aAnimation);
+      const wr::LayoutVector2D& aAppliedOffset, wr::SpatialTreeItemKey aKey);

  Maybe<wr::WrSpatialId> GetScrollIdForDefinedScrollLayer(
      layers::ScrollableLayerGuid::ViewID aViewId) const;
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@ -2756,19 +2756,8 @@ pub extern "C" fn wr_dp_define_sticky_frame(
    horizontal_bounds: StickyOffsetBounds,
    applied_offset: LayoutVector2D,
    key: SpatialTreeItemKey,
-    animation: *const WrAnimationProperty
 ) -> WrSpatialId {
    assert!(unsafe { is_in_main_thread() });
-    let anim = unsafe { animation.as_ref() };
-    let transform = anim.map(|anim| {
-      debug_assert!(anim.id > 0);
-      match anim.effect_type {
-        WrAnimationType::Transform => {
-          PropertyBinding::Binding(PropertyBindingKey::new(anim.id), LayoutTransform::identity())
-        },
-        _ => unreachable!("sticky elements can only have a transform animated")
-      }
-    });
    let spatial_id = state.frame_builder.dl_builder.define_sticky_frame(
        parent_spatial_id.to_webrender(state.pipeline_id),
        content_rect,
@ -2782,7 +2771,6 @@ pub extern "C" fn wr_dp_define_sticky_frame(
        horizontal_bounds,
        applied_offset,
        key,
-        transform
    );

    WrSpatialId { id: spatial_id.0 }
--- a/gfx/wr/examples/scrolling.rs
+++ b/gfx/wr/examples/scrolling.rs
@ -164,7 +164,6 @@ impl Example for App {
                StickyOffsetBounds::new(0.0, 0.0),
                LayoutVector2D::new(0.0, 0.0),
                SpatialTreeItemKey::new(0, 2),
-                None,
            );

            let info = CommonItemProperties::new(
--- a/gfx/wr/webrender/src/scene_building.rs
+++ b/gfx/wr/webrender/src/scene_building.rs
@ -1041,7 +1041,6 @@ impl<'a> SceneBuilder<'a> {
            info.vertical_offset_bounds,
            info.horizontal_offset_bounds,
            info.previously_applied_offset,
-            info.transform,
        );

        let index = self.spatial_tree.add_sticky_frame(
--- a/gfx/wr/webrender/src/spatial_node.rs
+++ b/gfx/wr/webrender/src/spatial_node.rs
@ -518,46 +518,17 @@ impl SpatialNode {
                self.viewport_transform = cs_scale_offset;
                self.content_transform = cs_scale_offset;
            }
-            SpatialNodeType::StickyFrame(ref mut info) => {
-                let animated_offset = if let Some(transform_binding) = info.transform {
-                  let transform = scene_properties.resolve_layout_transform(&transform_binding);
-                  match ScaleOffset::from_transform(&transform) {
-                    Some(ref scale_offset) => {
-                      debug_assert!(scale_offset.scale == Vector2D::new(1.0, 1.0),
-                                    "Can only animate a translation on sticky elements");
-                      LayoutVector2D::from_untyped(scale_offset.offset)
-                    }
-                    None => {
-                      debug_assert!(false, "Can only animate a translation on sticky elements");
-                      LayoutVector2D::zero()
-                    }
-                  }
-                } else {
-                  LayoutVector2D::zero()
-                };
-
-                let sticky_offset = Self::calculate_sticky_offset(
+            _ => {
+                // We calculate this here to avoid a double-borrow later.
+                let sticky_offset = self.calculate_sticky_offset(
                    &state.nearest_scrolling_ancestor_offset,
                    &state.nearest_scrolling_ancestor_viewport,
-                    info,
                );

                // The transformation for the bounds of our viewport is the parent reference frame
                // transform, plus any accumulated scroll offset from our parents, plus any offset
                // provided by our own sticky positioning.
-                let accumulated_offset = state.parent_accumulated_scroll_offset + sticky_offset + animated_offset;
-                self.viewport_transform = state.coordinate_system_relative_scale_offset
-                    .offset(snap_offset(accumulated_offset, state.coordinate_system_relative_scale_offset.scale).to_untyped());
-                self.content_transform = self.viewport_transform;
-
-                info.current_offset = sticky_offset + animated_offset;
-
-                self.coordinate_system_id = state.current_coordinate_system_id;
-            }
-            SpatialNodeType::ScrollFrame(_) => {
-                // The transformation for the bounds of our viewport is the parent reference frame
-                // transform, plus any accumulated scroll offset from our parents.
-                let accumulated_offset = state.parent_accumulated_scroll_offset;
+                let accumulated_offset = state.parent_accumulated_scroll_offset + sticky_offset;
                self.viewport_transform = state.coordinate_system_relative_scale_offset
                    .offset(snap_offset(accumulated_offset, state.coordinate_system_relative_scale_offset.scale).to_untyped());

@ -567,8 +538,12 @@ impl SpatialNode {
                self.content_transform = state.coordinate_system_relative_scale_offset
                    .offset(snap_offset(added_offset, state.coordinate_system_relative_scale_offset.scale).to_untyped());

+                if let SpatialNodeType::StickyFrame(ref mut info) = self.node_type {
+                    info.current_offset = sticky_offset;
+                }
+
                self.coordinate_system_id = state.current_coordinate_system_id;
-          }
+            }
        }

        //TODO: remove the field entirely?
@ -580,10 +555,15 @@ impl SpatialNode {
    }

    fn calculate_sticky_offset(
+        &self,
        viewport_scroll_offset: &LayoutVector2D,
        viewport_rect: &LayoutRect,
-        info: &StickyFrameInfo
    ) -> LayoutVector2D {
+        let info = match self.node_type {
+            SpatialNodeType::StickyFrame(ref info) => info,
+            _ => return LayoutVector2D::zero(),
+        };
+
        if info.margins.top.is_none() && info.margins.bottom.is_none() &&
            info.margins.left.is_none() && info.margins.right.is_none() {
            return LayoutVector2D::zero();
@ -905,13 +885,12 @@ pub struct ReferenceFrameInfo {
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub struct StickyFrameInfo {
-  pub margins: SideOffsets2D<Option<f32>, LayoutPixel>,
-  pub frame_rect: LayoutRect,
+    pub frame_rect: LayoutRect,
+    pub margins: SideOffsets2D<Option<f32>, LayoutPixel>,
    pub vertical_offset_bounds: StickyOffsetBounds,
    pub horizontal_offset_bounds: StickyOffsetBounds,
    pub previously_applied_offset: LayoutVector2D,
    pub current_offset: LayoutVector2D,
-    pub transform: Option<PropertyBinding<LayoutTransform>>,
 }

 impl StickyFrameInfo {
@ -920,8 +899,7 @@ impl StickyFrameInfo {
        margins: SideOffsets2D<Option<f32>, LayoutPixel>,
        vertical_offset_bounds: StickyOffsetBounds,
        horizontal_offset_bounds: StickyOffsetBounds,
-        previously_applied_offset: LayoutVector2D,
-        transform: Option<PropertyBinding<LayoutTransform>>,
+        previously_applied_offset: LayoutVector2D
    ) -> StickyFrameInfo {
        StickyFrameInfo {
            frame_rect,
@ -930,7 +908,6 @@ impl StickyFrameInfo {
            horizontal_offset_bounds,
            previously_applied_offset,
            current_offset: LayoutVector2D::zero(),
-            transform,
        }
    }
 }
--- a/gfx/wr/webrender_api/src/display_item.rs
+++ b/gfx/wr/webrender_api/src/display_item.rs
@ -323,9 +323,6 @@ pub struct StickyFrameDescriptor {

    /// A unique (per-pipeline) key for this spatial that is stable across display lists.
    pub key: SpatialTreeItemKey,
-
-    /// A property binding that we use to store an animation ID for APZ
-    pub transform: Option<PropertyBinding<LayoutTransform>>,
 }

 #[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, PeekPoke)]
--- a/gfx/wr/webrender_api/src/display_list.rs
+++ b/gfx/wr/webrender_api/src/display_list.rs
@ -2040,9 +2040,6 @@ impl DisplayListBuilder {
        horizontal_offset_bounds: di::StickyOffsetBounds,
        previously_applied_offset: LayoutVector2D,
        key: di::SpatialTreeItemKey,
-        // TODO: The caller only ever passes an identity transform.
-        // Could we pass just an (optional) animation id instead?
-        transform: Option<PropertyBinding<LayoutTransform>>
    ) -> di::SpatialId {
        let id = self.generate_spatial_index();
        let current_offset = self.current_offset(parent_spatial_id);
@ -2062,7 +2059,6 @@ impl DisplayListBuilder {
            horizontal_offset_bounds,
            previously_applied_offset,
            key,
-            transform,
        });

        self.push_spatial_tree_item(&descriptor);
--- a/gfx/wr/wrench/src/yaml_frame_reader.rs
+++ b/gfx/wr/wrench/src/yaml_frame_reader.rs
@ -1685,7 +1685,6 @@ impl YamlFrameReader {
            yaml["horizontal-offset-bounds"].as_sticky_offset_bounds(),
            yaml["previously-applied-offset"].as_vector().unwrap_or_else(LayoutVector2D::zero),
            self.next_spatial_key(),
-            None,
        );

        if let Some(numeric_id) = numeric_id {
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@ -5207,7 +5207,7 @@ bool nsDisplayOwnLayer::CreateWebRenderCommands(
  Maybe<wr::WrAnimationProperty> prop;
  bool needsProp = aManager->LayerManager()->AsyncPanZoomEnabled() &&
                   (IsScrollThumbLayer() || IsZoomingLayer() ||
-                    ShouldGetFixedAnimationId() ||
+                    ShouldGetFixedOrStickyAnimationId() ||
                    (IsRootScrollbarContainer() && HasDynamicToolbar()));

  if (needsProp) {
@ -5234,7 +5234,7 @@ bool nsDisplayOwnLayer::CreateWebRenderCommands(
    params.prim_flags |= wr::PrimitiveFlags::IS_SCROLLBAR_CONTAINER;
  }
  if (IsZoomingLayer() ||
-      (ShouldGetFixedAnimationId() ||
+      (ShouldGetFixedOrStickyAnimationId() ||
       (IsRootScrollbarContainer() && HasDynamicToolbar()))) {
    params.is_2d_scale_translation = true;
    params.should_snap = true;
@ -5250,8 +5250,9 @@ bool nsDisplayOwnLayer::CreateWebRenderCommands(

 bool nsDisplayOwnLayer::UpdateScrollData(WebRenderScrollData* aData,
                                         WebRenderLayerScrollData* aLayerData) {
-  bool isRelevantToApz = (IsScrollThumbLayer() || IsScrollbarContainer() ||
-                          IsZoomingLayer() || ShouldGetFixedAnimationId());
+  bool isRelevantToApz =
+      (IsScrollThumbLayer() || IsScrollbarContainer() || IsZoomingLayer() ||
+       ShouldGetFixedOrStickyAnimationId());

  if (!isRelevantToApz) {
    return false;
@ -5266,11 +5267,16 @@ bool nsDisplayOwnLayer::UpdateScrollData(WebRenderScrollData* aData,
    return true;
  }

-  if (IsFixedPositionLayer() && ShouldGetFixedAnimationId()) {
+  if (IsFixedPositionLayer() && ShouldGetFixedOrStickyAnimationId()) {
    aLayerData->SetFixedPositionAnimationId(mWrAnimationId);
    return true;
  }

+  if (IsStickyPositionLayer() && ShouldGetFixedOrStickyAnimationId()) {
+    aLayerData->SetStickyPositionAnimationId(mWrAnimationId);
+    return true;
+  }
+
  MOZ_ASSERT(IsScrollbarContainer() || IsScrollThumbLayer());

  aLayerData->SetScrollbarData(mScrollbarData);
@ -5452,7 +5458,7 @@ bool nsDisplayFixedPosition::UpdateScrollData(
  return true;
 }

-bool nsDisplayFixedPosition::ShouldGetFixedAnimationId() {
+bool nsDisplayFixedPosition::ShouldGetFixedOrStickyAnimationId() {
 #if defined(MOZ_WIDGET_ANDROID)
  return mFrame->PresContext()->IsRootContentDocumentCrossProcess() &&
         nsLayoutUtils::ScrollIdForRootScrollFrame(mFrame->PresContext()) ==
@ -5516,8 +5522,7 @@ nsDisplayStickyPosition::nsDisplayStickyPosition(
    : nsDisplayOwnLayer(aBuilder, aFrame, aList, aActiveScrolledRoot),
      mContainerASR(aContainerASR),
      mClippedToDisplayPort(aClippedToDisplayPort),
-      mShouldFlatten(false),
-      mWrStickyAnimationId(0) {
+      mShouldFlatten(false) {
  MOZ_COUNT_CTOR(nsDisplayStickyPosition);
 }

@ -5733,27 +5738,12 @@ bool nsDisplayStickyPosition::CreateWebRenderCommands(
    wr::LayoutVector2D applied = {
        NSAppUnitsToFloatPixels(appliedOffset.x, auPerDevPixel),
        NSAppUnitsToFloatPixels(appliedOffset.y, auPerDevPixel)};
-    bool needsProp = ShouldGetStickyAnimationId();
-    Maybe<wr::WrAnimationProperty> prop;
-    auto spatialKey = wr::SpatialKey(uint64_t(mFrame), GetPerFrameKey(),
-                                     wr::SpatialKeyKind::Sticky);
-    if (needsProp) {
-      RefPtr<WebRenderAPZAnimationData> animationData =
-          aManager->CommandBuilder()
-              .CreateOrRecycleWebRenderUserData<WebRenderAPZAnimationData>(
-                  this);
-      mWrStickyAnimationId = animationData->GetAnimationId();
-
-      prop.emplace();
-      prop->id = mWrStickyAnimationId;
-      prop->key = spatialKey;
-      prop->effect_type = wr::WrAnimationType::Transform;
-    }
    wr::WrSpatialId spatialId = aBuilder.DefineStickyFrame(
        wr::ToLayoutRect(bounds), topMargin.ptrOr(nullptr),
        rightMargin.ptrOr(nullptr), bottomMargin.ptrOr(nullptr),
-        leftMargin.ptrOr(nullptr), vBounds, hBounds, applied, spatialKey,
-        prop.ptrOr(nullptr));
+        leftMargin.ptrOr(nullptr), vBounds, hBounds, applied,
+        wr::SpatialKey(uint64_t(mFrame), GetPerFrameKey(),
+                       wr::SpatialKeyKind::Sticky));

    saccHelper.emplace(aBuilder, spatialId);
    aManager->CommandBuilder().PushOverrideForASR(mContainerASR, spatialId);
@ -5822,10 +5812,6 @@ bool nsDisplayStickyPosition::UpdateScrollData(
                                               ->GetContent());
      aLayerData->SetStickyPositionScrollContainerId(scrollId);
    }
-
-    if (ShouldGetStickyAnimationId()) {
-      aLayerData->SetStickyPositionAnimationId(mWrStickyAnimationId);
-    }
  }
  // Return true if either there is a dynamic toolbar affecting this sticky
  // item or the OwnLayer base implementation returns true for some other
@ -5835,9 +5821,19 @@ bool nsDisplayStickyPosition::UpdateScrollData(
  return ret;
 }

-bool nsDisplayStickyPosition::ShouldGetStickyAnimationId() const {
+bool nsDisplayStickyPosition::ShouldGetFixedOrStickyAnimationId() {
 #if defined(MOZ_WIDGET_ANDROID)
-  return HasDynamicToolbar();  // also implies being in the cross-process RCD
+  if (HasDynamicToolbar()) {  // also implies being in the cross-process RCD
+    StickyScrollContainer* stickyScrollContainer = GetStickyScrollContainer();
+    if (stickyScrollContainer) {
+      ScrollableLayerGuid::ViewID scrollId =
+          nsLayoutUtils::FindOrCreateIDFor(stickyScrollContainer->ScrollFrame()
+                                               ->GetScrolledFrame()
+                                               ->GetContent());
+      return nsLayoutUtils::ScrollIdForRootScrollFrame(mFrame->PresContext()) ==
+             scrollId;
+    }
+  }
 #endif
  return false;
 }
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@ -5475,7 +5475,7 @@ class nsDisplayOwnLayer : public nsDisplayWrapList {
  bool IsFixedPositionLayer() const;
  bool IsStickyPositionLayer() const;
  bool HasDynamicToolbar() const;
-  virtual bool ShouldGetFixedAnimationId() { return false; }
+  virtual bool ShouldGetFixedOrStickyAnimationId() { return false; }

  bool CreatesStackingContextHelper() override { return true; }

@ -5491,16 +5491,6 @@ class nsDisplayOwnLayer : public nsDisplayWrapList {
   */
  layers::ScrollbarData mScrollbarData;
  bool mForceActive;
-
-  // Used for APZ to animate this layer for purposes such as
-  // pinch-zooming or scrollbar thumb movement. Note that setting this
-  // creates a WebRender ReferenceFrame spatial node, and should only
-  // be used for display items that establish a Gecko reference frame
-  // as well (or leaf items like scrollbar thumb nodes where it does not
-  // matter).
-  // FIXME: This is currently also used for adjusting position:fixed items
-  // for dynamic toolbar movement. This may be a problem as position:fixed
-  // items do not establish Gecko reference frames.
  uint64_t mWrAnimationId;
 };

@ -5560,8 +5550,7 @@ class nsDisplayStickyPosition : public nsDisplayOwnLayer {
      : nsDisplayOwnLayer(aBuilder, aOther),
        mContainerASR(aOther.mContainerASR),
        mClippedToDisplayPort(aOther.mClippedToDisplayPort),
-        mShouldFlatten(false),
-        mWrStickyAnimationId(0) {
+        mShouldFlatten(false) {
    MOZ_COUNT_CTOR(nsDisplayStickyPosition);
  }

@ -5586,6 +5575,7 @@ class nsDisplayStickyPosition : public nsDisplayOwnLayer {

  bool UpdateScrollData(layers::WebRenderScrollData* aData,
                        layers::WebRenderLayerScrollData* aLayerData) override;
+  bool ShouldGetFixedOrStickyAnimationId() override;

  const ActiveScrolledRoot* GetContainerASR() const { return mContainerASR; }

@ -5601,8 +5591,6 @@ class nsDisplayStickyPosition : public nsDisplayOwnLayer {
    return mShouldFlatten;
  }

-  bool ShouldGetStickyAnimationId() const;
-
 private:
  NS_DISPLAY_ALLOW_CLONING()

@ -5632,13 +5620,6 @@ class nsDisplayStickyPosition : public nsDisplayOwnLayer {

  // True if this item should be flattened away.
  bool mShouldFlatten;
-
-  // Used for APZ to animate the sticky element in the compositor
-  // for purposes such as dynamic toolbar movement and (in the future)
-  // overscroll-related adjustment. Unlike nsDisplayOwnLayer::mWrAnimationId,
-  // this does not create a WebRender ReferenceFrame, which is important
-  // because sticky elements do not establish Gecko reference frames either.
-  uint64_t mWrStickyAnimationId;
 };

 class nsDisplayFixedPosition : public nsDisplayOwnLayer {
@ -5680,7 +5661,7 @@ class nsDisplayFixedPosition : public nsDisplayOwnLayer {
      nsDisplayListBuilder* aDisplayListBuilder) override;
  bool UpdateScrollData(layers::WebRenderScrollData* aData,
                        layers::WebRenderLayerScrollData* aLayerData) override;
-  bool ShouldGetFixedAnimationId() override;
+  bool ShouldGetFixedOrStickyAnimationId() override;
  void WriteDebugInfo(std::stringstream& aStream) override;

 protected: