Bug 1878294. Teach the scale tracking in ActiveLayerTracker to understand svg transforms. r=longsonr

The bug comes about because the svg transform is changed, so IncrementScaleRestyleCountIfNeeded gets called. It determines that there is no transform because it doesn't check svg transforms, so it incorrectly concludes that there was a transform but now it was removed, and so marks the scale as having been changed. Then in ChooseScale in StackingContextHelper, because we think the scale is being animated, we clamp the allowed scale factor based on the ratio of the viewport and the size of the svg. So avoiding marking this as incorrectly having an animating scale fixes the bug.

A simpler fix for this bug would be to check if the mPreviousTransformScale was Nothing() and only marked as mutated if it was Some.

This fix is a bit more complicated but it allows us to detect svg transform scales changing.

Differential Revision: https://phabricator.services.mozilla.com/D200579
This commit is contained in:
Timothy Nikkel 2024-02-28 13:37:38 +00:00
parent c5e0ded9c9
commit cca7ff4dbc
5 changed files with 82 additions and 13 deletions

View File

@ -218,23 +218,47 @@ void ActiveLayerTracker::TransferActivityToFrame(nsIContent* aContent,
static void IncrementScaleRestyleCountIfNeeded(nsIFrame* aFrame,
LayerActivity* aActivity) {
// This function is basically a simplified copy of
// nsDisplayTransform::GetResultingTransformMatrixInternal.
Matrix svgTransform, parentsChildrenOnlyTransform;
const bool hasSVGTransforms =
aFrame->HasAnyStateBits(NS_FRAME_MAY_BE_TRANSFORMED) &&
aFrame->IsSVGTransformed(&svgTransform, &parentsChildrenOnlyTransform);
const nsStyleDisplay* display = aFrame->StyleDisplay();
if (!display->HasTransformProperty() && !display->HasIndividualTransform() &&
display->mOffsetPath.IsNone()) {
// The transform was removed.
aActivity->mPreviousTransformScale = Nothing();
IncrementMutationCount(
&aActivity->mRestyleCounts[LayerActivity::ACTIVITY_SCALE]);
if (!aFrame->HasAnyStateBits(NS_FRAME_MAY_BE_TRANSFORMED) ||
(!display->HasTransformProperty() && !display->HasIndividualTransform() &&
display->mOffsetPath.IsNone() && !hasSVGTransforms)) {
if (aActivity->mPreviousTransformScale.isSome()) {
// The transform was removed.
aActivity->mPreviousTransformScale = Nothing();
IncrementMutationCount(
&aActivity->mRestyleCounts[LayerActivity::ACTIVITY_SCALE]);
}
return;
}
// Compute the new scale due to the CSS transform property.
// Note: Motion path doesn't contribute to scale factor. (It only has 2d
// translate and 2d rotate, so we use Nothing() for it.)
nsStyleTransformMatrix::TransformReferenceBox refBox(aFrame);
Matrix4x4 transform = nsStyleTransformMatrix::ReadTransforms(
display->mTranslate, display->mRotate, display->mScale, nullptr,
display->mTransform, refBox, AppUnitsPerCSSPixel());
Matrix4x4 transform;
if (aFrame->IsCSSTransformed()) {
// Compute the new scale due to the CSS transform property.
// Note: Motion path doesn't contribute to scale factor. (It only has 2d
// translate and 2d rotate, so we use Nothing() for it.)
nsStyleTransformMatrix::TransformReferenceBox refBox(aFrame);
transform = nsStyleTransformMatrix::ReadTransforms(
display->mTranslate, display->mRotate, display->mScale, nullptr,
display->mTransform, refBox, AppUnitsPerCSSPixel());
} else if (hasSVGTransforms) {
transform = Matrix4x4::From2D(svgTransform);
}
const bool parentHasChildrenOnlyTransform =
hasSVGTransforms && !parentsChildrenOnlyTransform.IsIdentity();
if (parentHasChildrenOnlyTransform) {
transform *= Matrix4x4::From2D(parentsChildrenOnlyTransform);
}
Matrix transform2D;
if (!transform.Is2D(&transform2D)) {
// We don't attempt to handle 3D transforms; just assume the scale changed.

View File

@ -6194,6 +6194,9 @@ Matrix4x4 nsDisplayTransform::GetResultingTransformMatrixInternal(
NS_ASSERTION(frame || !(aFlags & INCLUDE_PERSPECTIVE),
"Must have a frame to compute perspective!");
// IncrementScaleRestyleCountIfNeeded in ActiveLayerTracker.cpp is a
// simplified copy of this function.
// Get the underlying transform matrix:
/* Get the matrix, then change its basis to factor in the origin. */

View File

@ -0,0 +1,8 @@
<html>
<svg width="400" height="400">
<g id="g">
<circle cx="300" cy="300" r="40" style="fill: red;" transform="translate(-108213.2,-108213.2) scale(399)"></circle>
</g>
</svg>
</html>

View File

@ -0,0 +1,33 @@
<html class="reftest-wait">
<svg width="400" height="400">
<g id="g">
<circle cx="300" cy="300" r="40" style="fill: red;"></circle>
</g>
</svg>
<script>
const g = document.getElementById("g")
let tx = -108213.2;
let ty = -108213.2;
let k = 399;
let stepSize = 1;
let num = 0;
const move = () => {
stepSize = -1 * stepSize;
tx += 0.0001 * stepSize;
ty += 0.0001 * stepSize;
g.setAttribute("transform", `translate(${tx},${ty}) scale(${k})`)
window.setTimeout(move, 100);
num++;
if (num == 5) {
document.documentElement.className = "";
}
}
move();
</script>
</html>

View File

@ -2164,3 +2164,4 @@ fuzzy-if(!useDrawSnapshot,18-19,294-322) == 1840747-1.html about:blank
# through, which is unexpected and is responsible for the 255-255 difference
# here. That's tracked in bug 1840747.
fuzzy-if(!useDrawSnapshot&&!swgl,254-255,110-121) fuzzy-if(useDrawSnapshot,18-18,93-93) fuzzy-if(swgl,19-19,58-58) == 1841355-1.html about:blank
skip-if(((AddressSanitizer||ThreadSanitizer)&&gtkWidget)||(isDebugBuild&&Android)) fuzzy(0-123,0-1425) == 1878294-1.html 1878294-1-ref.html