From c0a23f0579cf3dc8f5bae0fc1efc8308e51458f2 Mon Sep 17 00:00:00 2001 From: Fedor Date: Thu, 5 Sep 2019 20:09:38 +0300 Subject: [PATCH] SVG placement misaligned. --- gfx/thebes/gfxMatrix.h | 12 +++++ layout/base/nsDisplayList.cpp | 41 ++++++++------ .../reftests/svg/css-transform-svg-ref.html | 10 ++++ layout/reftests/svg/css-transform-svg.html | 13 +++++ layout/reftests/svg/reftest.list | 2 + ...svg-blurry-with-subpixel-position-ref.html | 13 +++++ .../svg-blurry-with-subpixel-position.html | 13 +++++ layout/svg/nsSVGOuterSVGFrame.cpp | 53 ++++++++++++------- 8 files changed, 122 insertions(+), 35 deletions(-) create mode 100644 layout/reftests/svg/css-transform-svg-ref.html create mode 100644 layout/reftests/svg/css-transform-svg.html create mode 100644 layout/reftests/svg/svg-blurry-with-subpixel-position-ref.html create mode 100644 layout/reftests/svg/svg-blurry-with-subpixel-position.html diff --git a/gfx/thebes/gfxMatrix.h b/gfx/thebes/gfxMatrix.h index 9282a22db..4f92262cc 100644 --- a/gfx/thebes/gfxMatrix.h +++ b/gfx/thebes/gfxMatrix.h @@ -112,6 +112,18 @@ public: _31 == 0.0 && _32 == 0.0; } + /* Returns true if the matrix is a rectilinear transformation (i.e. + * grid-aligned rectangles are transformed to grid-aligned rectangles) + */ + bool IsRectilinear() const { + if (FuzzyEqual(_12, 0) && FuzzyEqual(_21, 0)) { + return true; + } else if (FuzzyEqual(_22, 0) && FuzzyEqual(_11, 0)) { + return true; + } + return false; + } + /** * Inverts this matrix, if possible. Otherwise, the matrix is left * unchanged. diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index e35e027e3..e22230b41 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -5708,7 +5708,7 @@ nsDisplayTransform::GetDeltaToTransformOrigin(const nsIFrame* aFrame, } /* Allows us to access dimension getters by index. */ - float coords[2]; + float transformOrigin[2]; TransformReferenceBox::DimensionGetter dimensionGetter[] = { &TransformReferenceBox::Width, &TransformReferenceBox::Height }; TransformReferenceBox::DimensionGetter offsetGetter[] = @@ -5718,33 +5718,33 @@ nsDisplayTransform::GetDeltaToTransformOrigin(const nsIFrame* aFrame, /* If the transform-origin specifies a percentage, take the percentage * of the size of the box. */ - const nsStyleCoord &coord = display->mTransformOrigin[index]; - if (coord.GetUnit() == eStyleUnit_Calc) { - const nsStyleCoord::Calc *calc = coord.GetCalcValue(); - coords[index] = + const nsStyleCoord &originValue = display->mTransformOrigin[index]; + if (originValue.GetUnit() == eStyleUnit_Calc) { + const nsStyleCoord::Calc *calc = originValue.GetCalcValue(); + transformOrigin[index] = NSAppUnitsToFloatPixels((refBox.*dimensionGetter[index])(), aAppUnitsPerPixel) * calc->mPercent + NSAppUnitsToFloatPixels(calc->mLength, aAppUnitsPerPixel); - } else if (coord.GetUnit() == eStyleUnit_Percent) { - coords[index] = + } else if (originValue.GetUnit() == eStyleUnit_Percent) { + transformOrigin[index] = NSAppUnitsToFloatPixels((refBox.*dimensionGetter[index])(), aAppUnitsPerPixel) * - coord.GetPercentValue(); + originValue.GetPercentValue(); } else { - MOZ_ASSERT(coord.GetUnit() == eStyleUnit_Coord, "unexpected unit"); - coords[index] = - NSAppUnitsToFloatPixels(coord.GetCoordValue(), aAppUnitsPerPixel); + MOZ_ASSERT(originValue.GetUnit() == eStyleUnit_Coord, "unexpected unit"); + transformOrigin[index] = + NSAppUnitsToFloatPixels(originValue.GetCoordValue(), aAppUnitsPerPixel); } if (aFrame->GetStateBits() & NS_FRAME_SVG_LAYOUT) { // SVG frames (unlike other frames) have a reference box that can be (and // typically is) offset from the TopLeft() of the frame. We need to // account for that here. - coords[index] += + transformOrigin[index] += NSAppUnitsToFloatPixels((refBox.*offsetGetter[index])(), aAppUnitsPerPixel); } } - return Point3D(coords[0], coords[1], + return Point3D(transformOrigin[0], transformOrigin[1], NSAppUnitsToFloatPixels(display->mTransformOrigin[2].GetCoordValue(), aAppUnitsPerPixel)); } @@ -5917,6 +5917,17 @@ nsDisplayTransform::GetResultingTransformMatrixInternal(const FrameTransformProp frame && frame->IsSVGTransformed(&svgTransform, &transformFromSVGParent); bool hasTransformFromSVGParent = hasSVGTransforms && !transformFromSVGParent.IsIdentity(); + + bool shouldRound = true; + + // An SVG frame should not have its translation rounded. + // Note it's possible that the SVG frame doesn't have an SVG + // transform but only has a CSS transform. + if (frame && frame->HasAnyStateBits(NS_FRAME_SVG_LAYOUT) && + !(frame->GetType() == nsGkAtoms::svgOuterSVGAnonChildFrame)) { + shouldRound = false; + } + /* Transformed frames always have a transform, or are preserving 3d (and might still have perspective!) */ if (aProperties.mTransformList) { result = nsStyleTransformMatrix::ReadTransforms(aProperties.mTransformList->mHead, @@ -5994,7 +6005,7 @@ nsDisplayTransform::GetResultingTransformMatrixInternal(const FrameTransformProp // Otherwise we need to manually translate into our parent's coordinate // space. if (frame->IsTransformed()) { - nsLayoutUtils::PostTranslate(result, frame->GetPosition(), aAppUnitsPerPixel, !hasSVGTransforms); + nsLayoutUtils::PostTranslate(result, frame->GetPosition(), aAppUnitsPerPixel, shouldRound); } Matrix4x4 parent = GetResultingTransformMatrixInternal(props, @@ -6005,7 +6016,7 @@ nsDisplayTransform::GetResultingTransformMatrixInternal(const FrameTransformProp } if (aFlags & OFFSET_BY_ORIGIN) { - nsLayoutUtils::PostTranslate(result, aOrigin, aAppUnitsPerPixel, !hasSVGTransforms); + nsLayoutUtils::PostTranslate(result, aOrigin, aAppUnitsPerPixel, shouldRound); } return result; diff --git a/layout/reftests/svg/css-transform-svg-ref.html b/layout/reftests/svg/css-transform-svg-ref.html new file mode 100644 index 000000000..6167442e7 --- /dev/null +++ b/layout/reftests/svg/css-transform-svg-ref.html @@ -0,0 +1,10 @@ + + + + + + diff --git a/layout/reftests/svg/css-transform-svg.html b/layout/reftests/svg/css-transform-svg.html new file mode 100644 index 000000000..c1c63a839 --- /dev/null +++ b/layout/reftests/svg/css-transform-svg.html @@ -0,0 +1,13 @@ + + + + + + diff --git a/layout/reftests/svg/reftest.list b/layout/reftests/svg/reftest.list index 096628681..21e1c68a1 100644 --- a/layout/reftests/svg/reftest.list +++ b/layout/reftests/svg/reftest.list @@ -140,6 +140,7 @@ random == dynamic-use-nested-01b.svg dynamic-use-nested-01-ref.svg == fragmentIdentifier-01.xhtml pass.svg == linked-filter-01.svg pass.svg == linked-pattern-01.svg pass.svg +pref(layout.css.devPixelsPerPx,"1.0") == svg-blurry-with-subpixel-position.html svg-blurry-with-subpixel-position-ref.html == use-01.svg pass.svg == use-01-extref.svg pass.svg == use-02-extref.svg use-02-extref-ref.svg @@ -371,6 +372,7 @@ fuzzy-if(skiaContent,1,610) == textPath-03.svg pass.svg == text-white-space-01.svg text-white-space-01-ref.svg == thin-stroke-01.svg pass.svg == zero-stroke-01.svg pass.svg +== css-transform-svg.html css-transform-svg-ref.html == tspan-dxdy-01.svg tspan-dxdy-ref.svg == tspan-dxdy-02.svg tspan-dxdy-ref.svg == tspan-dxdy-03.svg tspan-dxdy-ref.svg diff --git a/layout/reftests/svg/svg-blurry-with-subpixel-position-ref.html b/layout/reftests/svg/svg-blurry-with-subpixel-position-ref.html new file mode 100644 index 000000000..c315509d7 --- /dev/null +++ b/layout/reftests/svg/svg-blurry-with-subpixel-position-ref.html @@ -0,0 +1,13 @@ + + + + + + + diff --git a/layout/reftests/svg/svg-blurry-with-subpixel-position.html b/layout/reftests/svg/svg-blurry-with-subpixel-position.html new file mode 100644 index 000000000..20bca7174 --- /dev/null +++ b/layout/reftests/svg/svg-blurry-with-subpixel-position.html @@ -0,0 +1,13 @@ + + + + + + + diff --git a/layout/svg/nsSVGOuterSVGFrame.cpp b/layout/svg/nsSVGOuterSVGFrame.cpp index e1b97bb40..b1ee54eb9 100644 --- a/layout/svg/nsSVGOuterSVGFrame.cpp +++ b/layout/svg/nsSVGOuterSVGFrame.cpp @@ -979,43 +979,56 @@ nsSVGOuterSVGAnonChildFrame::GetType() const return nsGkAtoms::svgOuterSVGAnonChildFrame; } -bool -nsSVGOuterSVGAnonChildFrame::IsSVGTransformed(Matrix* aOwnTransform, - Matrix* aFromParentTransform) const +static Matrix +ComputeOuterSVGAnonChildFrameTransform(const nsSVGOuterSVGAnonChildFrame* aFrame) { // Our elements 'transform' attribute is applied to our nsSVGOuterSVGFrame // parent, and the element's children-only transforms are applied to us, the // anonymous child frame. Since we are the child frame, we apply the // children-only transforms as if they are our own transform. - SVGSVGElement* content = static_cast(mContent); + SVGSVGElement* content = static_cast(aFrame->GetContent()); if (!content->HasChildrenOnlyTransform()) { - return false; + return Matrix(); } // Outer- doesn't use x/y, so we can pass eChildToUserSpace here. gfxMatrix ownMatrix = content->PrependLocalTransformsTo(gfxMatrix(), eChildToUserSpace); - if (ownMatrix.IsIdentity()) { - return false; + if (ownMatrix.HasNonTranslation()) { + // viewBox, currentScale and currentTranslate should only produce a + // rectilinear transform. + MOZ_ASSERT(ownMatrix.IsRectilinear(), + "Non-rectilinear transform will break the following logic"); + + // The nsDisplayTransform code will apply this transform to our frame, + // including to our frame position. We don't want our frame position to + // be scaled though, so we need to correct for that in the transform. + CSSPoint pos = CSSPixel::FromAppUnits(aFrame->GetPosition()); + CSSPoint scaledPos = CSSPoint(ownMatrix._11 * pos.x, ownMatrix._22 * pos.y); + CSSPoint deltaPos = scaledPos - pos; + ownMatrix *= gfxMatrix::Translation(-deltaPos.x, -deltaPos.y); } - if (aOwnTransform) { - if (ownMatrix.HasNonTranslation()) { - // Note: viewBox, currentScale and currentTranslate should only - // produce a rectilinear transform. - // The nsDisplayTransform code will apply this transform to our frame, - // including to our frame position. We don't want our frame position to - // be scaled though, so we need to correct for that in the transform. - CSSPoint pos = CSSPixel::FromAppUnits(GetPosition()); - CSSPoint scaledPos = CSSPoint(ownMatrix._11 * pos.x, ownMatrix._22 * pos.y); - CSSPoint deltaPos = scaledPos - pos; - ownMatrix *= gfxMatrix::Translation(-deltaPos.x, -deltaPos.y); - } + return gfx::ToMatrix(ownMatrix); +} - *aOwnTransform = gfx::ToMatrix(ownMatrix); +// We want this frame to be a reference frame. An easy way to achieve that is +// to always return true from this method, even for identity transforms. +// This frame being a reference frame ensures that the offset between this +// element and the parent reference frame is completely absorbed by the +// nsDisplayTransform that's created for this frame, and that this offset does +// not affect our descendants' transforms. Consequently, if the element +// moves, e.g. during scrolling, the transform matrices of our contents are +// unaffected. This simplifies invalidation. +bool +nsSVGOuterSVGAnonChildFrame::IsSVGTransformed(Matrix* aOwnTransform, + Matrix* aFromParentTransform) const +{ + if (aOwnTransform) { + *aOwnTransform = ComputeOuterSVGAnonChildFrameTransform(this); } return true;