Bug 217473 - CSS transform computed style should not reflect individual transform properties
Summary: CSS transform computed style should not reflect individual transform properties
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks: 178117
  Show dependency treegraph
 
Reported: 2020-10-08 07:03 PDT by Antoine Quint
Modified: 2020-10-09 09:27 PDT (History)
20 users (show)

See Also:


Attachments
Patch (18.96 KB, patch)
2020-10-08 07:08 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (19.82 KB, patch)
2020-10-08 12:39 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (18.90 KB, patch)
2020-10-08 23:48 PDT, Antoine Quint
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (19.03 KB, patch)
2020-10-09 00:10 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2020-10-08 07:03:55 PDT
CSS transform computed style should not reflect individual transform properties
Comment 1 Radar WebKit Bug Importer 2020-10-08 07:04:45 PDT
<rdar://problem/70091605>
Comment 2 Antoine Quint 2020-10-08 07:08:37 PDT
Created attachment 410837 [details]
Patch
Comment 3 Antoine Quint 2020-10-08 07:08:42 PDT
<rdar://problem/70091605>
Comment 4 EWS Watchlist 2020-10-08 07:09:28 PDT
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
Comment 5 Antoine Quint 2020-10-08 07:18:30 PDT
WPT PR is https://github.com/web-platform-tests/wpt/pull/26044.
Comment 6 Simon Fraser (smfr) 2020-10-08 08:28:06 PDT
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.
Comment 7 Antoine Quint 2020-10-08 12:39:46 PDT
Created attachment 410871 [details]
Patch
Comment 8 Simon Fraser (smfr) 2020-10-08 13:21:06 PDT
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
Comment 9 Antoine Quint 2020-10-08 23:48:36 PDT
Created attachment 410917 [details]
Patch
Comment 10 Antoine Quint 2020-10-09 00:10:21 PDT
Created attachment 410918 [details]
Patch
Comment 11 EWS 2020-10-09 09:26:59 PDT
Committed r268263: <https://trac.webkit.org/changeset/268263>

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