commit f326950b5e24534838fc57687b4c6dd467657668
parent 8100261ad60de25787b6e2f3cdb81d3a1ff7438e
Author: Boris Chiou <boris.chiou@gmail.com>
Date: Fri, 3 Oct 2025 20:30:50 +0000
Bug 1965924 - Add back the nsDisplayTransform for the VT captured element to make sure WR render it well. r=layout-reviewers,view-transitions-reviewers,emilio,nical
If we don't use it, WR may render it with a lower resolution (just like
doing an linear scaling on the image when using scale() transform).
Also, there is an edge case when using identity transform on the captured
element. In this case, the wrapper nsDisplayTransform doesn't push a
reference frame, so we don't generate a new coordinate system for this
capture frame. This makes the offset of the captured frame wrong. So we
push a reference frame always if it is transformed as well.
Differential Revision: https://phabricator.services.mozilla.com/D261685
Diffstat:
5 files changed, 98 insertions(+), 19 deletions(-)
diff --git a/gfx/webrender_bindings/WebRenderTypes.h b/gfx/webrender_bindings/WebRenderTypes.h
@@ -127,6 +127,7 @@ enum class SpatialKeyKind : uint32_t {
Sticky,
ImagePipeline,
APZ,
+ ViewTransition,
};
// Construct a unique, persistent spatial key based on the frame tree pointer,
diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp
@@ -3430,7 +3430,8 @@ void nsIFrame::BuildDisplayListForStackingContext(
// one and the inner container items will be unclipped.
ContainerItemType clipCapturedBy = ContainerItemType::None;
if (capturedByViewTransition) {
- clipCapturedBy = ContainerItemType::ViewTransitionCapture;
+ clipCapturedBy = isTransformed ? ContainerItemType::Transform
+ : ContainerItemType::ViewTransitionCapture;
} else if (useFixedPosition) {
clipCapturedBy = ContainerItemType::FixedPosition;
} else if (isTransformed) {
@@ -3718,6 +3719,36 @@ void nsIFrame::BuildDisplayListForStackingContext(
createdContainer = true;
}
+ // We build nsDisplayViewTransitionCapture here, and then use
+ // nsDisplayTransform to wrap it, to make sure we create the correct transform
+ // for the captured element and its descendants. This is necessary to make
+ // sure our captured element doesn't become blurry when using scale()
+ // transform.
+ //
+ // So the display list looks like this:
+ // nsDisplayTransform // For the captured element if it is transformed
+ // VTCapture // For the captured element
+ // ...
+ // Other display items // For the descendants of the captured element
+ // ...
+ // ...
+ //
+ // We intentionally use nsDisplayTransform to wrap the VTCapture (so it's
+ // different from opacity display item) because nsDisplayTransform may push a
+ // reference frame which creates a new coordinate system in WR. So it's just
+ // like a separator between the VTCapture the outside, if it is transformed.
+ if (capturedByViewTransition) {
+ resultList.AppendNewToTop<nsDisplayViewTransitionCapture>(
+ aBuilder, this, &resultList, nullptr, false);
+ createdContainer = true;
+ MarkAsIsolated();
+ // We don't want the capture to be clipped, so we do this _after_ building
+ // the wrapping item.
+ if (clipCapturedBy == ContainerItemType::ViewTransitionCapture) {
+ clipState.Restore();
+ }
+ }
+
// If we're going to apply a transformation and don't have preserve-3d set,
// wrap everything in an nsDisplayTransform. If there's nothing in the list,
// don't add anything.
@@ -3730,9 +3761,9 @@ void nsIFrame::BuildDisplayListForStackingContext(
// We also traverse into sublists created by nsDisplayWrapList, so that we
// find all the correct children.
//
- // We don't bother creating nsDisplayTransform and co for
- // view-transition-captured elements.
- if (isTransformed && !capturedByViewTransition) {
+ // We still need to create nsDisplayTransform to wrap the VT capture element
+ // to make sure WR renders it properly if it is transformed as well.
+ if (isTransformed) {
if (extend3DContext) {
// Install dummy nsDisplayTransform as a leaf containing
// descendants not participating this 3D rendering context.
@@ -3931,18 +3962,6 @@ void nsIFrame::BuildDisplayListForStackingContext(
MarkAsIsolated();
}
- if (capturedByViewTransition) {
- resultList.AppendNewToTop<nsDisplayViewTransitionCapture>(
- aBuilder, this, &resultList, nullptr, /* aIsRoot = */ false);
- createdContainer = true;
- MarkAsIsolated();
- // We don't want the capture to be clipped, so we do this _after_ building
- // the wrapping item.
- if (clipCapturedBy == ContainerItemType::ViewTransitionCapture) {
- clipState.Restore();
- }
- }
-
if (!isolated && localIsolationReasons != StackingContextBits::None) {
resultList.AppendToTop(nsDisplayBlendContainer::CreateForIsolation(
aBuilder, this, &resultList, containerItemASR,
diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp
@@ -5428,6 +5428,63 @@ bool nsDisplayViewTransitionCapture::CreateWebRenderCommands(
wr::StackingContextParams params;
params.clip =
wr::WrStackingContextClip::ClipChain(aBuilder.CurrentClipChainId());
+
+ // This is for the case if this frame is transformed. In this case, the wr
+ // display list looks like:
+ //
+ // PushStackingContext(...) // For the root
+ // ...
+ // nsDisplayTransform // For the captured element if it is transformed
+ // PushReferenceFrame // If the transform matrix is not identity
+ // PushStackingContext(...)
+ // VTCapture // For the captured element
+ // PushReferenceFrame
+ // PushStackingContext
+ // ...
+ // Other display items // For the descendants of the captured element
+ // ...
+ // PopStackingContext
+ // PopReferenceFrame
+ // PopStackingContext
+ // PopReferenceFrame
+ // ...
+ // PopStackingContext
+ //
+ // The wrapper, nsDisplayTransform, creates a new coordinate system whose
+ // origin is from ToReferenceFrame(mFrame). The VTCapture item is inside this
+ // nsDisplayTransform, so it's in this coordinate system. This works well if
+ // the transform matrix is not identity. However, if the transform matrix is
+ // identity, we don't create a new coordinate system (because we don't push a
+ // reference frame in WR), at this moment, the |ref_frame_offset| of the
+ // stacking context of this VTCapture becomes the |origin| of the
+ // nsDisplayTransform (see wr_dp_push_stacking_context() and
+ // push_stacking_context() for more details), so this offset may makes the
+ // position of this VTCapture wrong. (Note that the ToReferenceFrame(mFrame)
+ // of this VTCapture is the frame itself, but ToReferenceFrame(mFrame) of the
+ // nsDisplayTransform is its containing frame, e.g. viewport frame.)
+ //
+ // Therefore, we push a reference frame for this display item if the frame is
+ // transformed. This makes this VTCapture creates a new coordinate system, so
+ // its |ref_frame_offset| will be (0, 0), just fit the nsDisplayTransform's
+ // position.
+ //
+ // FIXME: We can avoid pushing a reference frame if the transform matrix is
+ // not identity, because we generate a new coordinate system already from the
+ // nsDisplayTransform (see nsDisplayTransform::CreateWebRenderCommands() for
+ // more details).
+ wr::WrTransformInfo info;
+ if (mFrame->IsTransformed()) {
+ // Use an identity matrix to force to push a reference frame to create a new
+ // coordinate system for view transition captured frame.
+ params.mTransformPtr = [&]() {
+ info.transform = wr::ToLayoutTransform(gfx::Matrix4x4());
+ info.key = wr::SpatialKey(uint64_t(mFrame), GetPerFrameKey(),
+ wr::SpatialKeyKind::ViewTransition);
+ return &info;
+ }();
+ params.reference_frame_kind = wr::WrReferenceFrameKind::Transform;
+ }
+
if (key) {
vt->UpdateActiveRectForCapturedFrame(capturedFrame, aSc.GetInheritedScale(),
captureRect);
@@ -6815,7 +6872,7 @@ bool nsDisplayTransform::CreateWebRenderCommands(
if (newTransformMatrix.IsIdentity()) {
// If the transform is an identity transform, strip it out so that WR
// doesn't turn this stacking context into a reference frame, as it
- // affects positioning. Bug 1345577 tracks a better fix.
+ // affects positioning.
transformForSC = nullptr;
// In ChooseScaleAndSetTransform, we round the offset from the reference
diff --git a/testing/web-platform/meta/css/css-view-transitions/content-with-transform-new-image.html.ini b/testing/web-platform/meta/css/css-view-transitions/content-with-transform-new-image.html.ini
@@ -1,2 +1,3 @@
[content-with-transform-new-image.html]
- expected: FAIL
+ fuzzy:
+ if os == "win": maxDifference=0-92;totalPixels=0-502
diff --git a/testing/web-platform/meta/css/css-view-transitions/content-with-transform-old-image.html.ini b/testing/web-platform/meta/css/css-view-transitions/content-with-transform-old-image.html.ini
@@ -1,2 +1,3 @@
[content-with-transform-old-image.html]
- expected: FAIL
+ fuzzy:
+ if os == "win": maxDifference=0-92;totalPixels=0-502
