Bug 217473

Summary: CSS transform computed style should not reflect individual transform properties
Product: WebKit Reporter: Antoine Quint <graouts>
Component: CSSAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, clopez, dino, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, graouts, gyuyoung.kim, kondapallykalyan, macpherson, menard, pdr, sabouhallawa, schenney, sergio, simon.fraser, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 178117    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch none

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].