Bug 217473 – CSS transform computed style should not reflect individual transform properties

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antoine Quint]

CSS transform computed style should not reflect individual transform properties

[Radar WebKit Bug Importer]

<rdar://problem/70091605>

[Antoine Quint]

Created attachment 410837 [details]
Patch

[Antoine Quint]

<rdar://problem/70091605>

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Antoine Quint]

WPT PR is https://github.com/web-platform-tests/wpt/pull/26044.

[Simon Fraser (smfr)]

Comment on attachment 410837 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=410837&action=review

> Source/WebCore/rendering/style/RenderStyle.h:660
> +    enum class TransformOperationExclusion : uint8_t {
> +        TransformOrigin = 1 << 0,
> +        Scale           = 1 << 1,
> +        Rotate          = 1 << 2,
> +        Translate       = 1 << 3
> +    };

I wonder if this would be cleaner if you did inclusion rather than exclusion.

[Antoine Quint]

Created attachment 410871 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 410871 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=410871&action=review

> Source/WebCore/rendering/RenderLayer.cpp:1319
> +TransformationMatrix RenderLayer::currentTransform(OptionSet<RenderStyle::TransformOperationInclusion> inclusions) const

I'd call these options, not inclusions.

> Source/WebCore/rendering/RenderLayer.cpp:1345
> +        renderBox.style().applyTransform(currTransform, pixelSnappedBorderRect, { RenderStyle::TransformOperationInclusion::Rotate, RenderStyle::TransformOperationInclusion::Scale, RenderStyle::TransformOperationInclusion::Translate });

Let's keep the order as translate, rotate, scale

> Source/WebCore/rendering/RenderLayer.h:798
> +    TransformationMatrix currentTransform(OptionSet<RenderStyle::TransformOperationInclusion> = { RenderStyle::TransformOperationInclusion::TransformOrigin, RenderStyle::TransformOperationInclusion::Rotate, RenderStyle::TransformOperationInclusion::Scale, RenderStyle::TransformOperationInclusion::Translate }) const;

Let's keep the order as translate, rotate, scale

> Source/WebCore/rendering/style/RenderStyle.h:660
> +    enum class TransformOperationInclusion : uint8_t {
> +        TransformOrigin = 1 << 0,
> +        Scale           = 1 << 1,
> +        Rotate          = 1 << 2,
> +        Translate       = 1 << 3
> +    };

You could add a constexpr OptionSet<TransformOperationInclusion> individualTransformFunctions = { TransformOperationInclusion::Translate, TransformOperationInclusion::Rotate, TransformOperationInclusion::Scale }

to avoid repeating these everywhere.

Use translate, rotate, scale order.

> Source/WebCore/svg/SVGTextElement.cpp:68
> +        return matrix;
> +    }

whitespace

[Antoine Quint]

Created attachment 410917 [details]
Patch

[Antoine Quint]

Created attachment 410918 [details]
Patch

[EWS]

Committed r268263: <https://trac.webkit.org/changeset/268263>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410918 [details].