Bug 237711 – [LBSE] Activate SVG transform support through layers

• Component: SVG
• Status: RESOLVED FIXED

[Nikolas Zimmermann]

Finally activate SVG transform support for containers & shapes in LBSE.

[Nikolas Zimmermann]

Created attachment 454350 [details]
Patch, v1

[Nikolas Zimmermann]

Delaying EWS submission until the dependency patches are landed.

[Nikolas Zimmermann]

Created attachment 454351 [details]
Patch, v2

[Nikolas Zimmermann]

Created attachment 454412 [details]
Patch, v3

[Nikolas Zimmermann]

Created attachment 454559 [details]
Patch, v6

The latest iteration of the patch introduces FloatOrLayoutRect, which either holds a FloatRect or LayoutRect (realized via variant<FloatRect, LayoutRect>) -- this avoids all LayoutRect(fromFloatRect) hacks, present in previous iterations of the patch and guarantees type-safety again. As benefit it allows us to detect if we need to pixel snap or not (SVG stores FloatRects, no pixel snapping should occour), simplifying quite a few parts in RenderLayer

[Nikolas Zimmermann]

Created attachment 454562 [details]
Patch, v4

[Nikolas Zimmermann]

(In reply to Nikolas Zimmermann from comment #5)
> Created attachment 454559 [details]
> Patch, v6
> 
> The latest iteration of the patch introduces FloatOrLayoutRect, which either
> holds a FloatRect or LayoutRect (realized via variant<FloatRect,
> LayoutRect>) -- this avoids all LayoutRect(fromFloatRect) hacks, present in
> previous iterations of the patch and guarantees type-safety again. As
> benefit it allows us to detect if we need to pixel snap or not (SVG stores
> FloatRects, no pixel snapping should occour), simplifying quite a few parts
> in RenderLayer
Please ignore this comment, this was supposed to go onto ticket 237554, not here.

[Nikolas Zimmermann]

Created attachment 454818 [details]
Patch, v5

[Radar WebKit Bug Importer]

<rdar://problem/90364832>

[Nikolas Zimmermann]

Created attachment 455227 [details]
Patch, v6

[Nikolas Zimmermann]

Created attachment 456619 [details]
Patch, v7

[Nikolas Zimmermann]

Created attachment 456754 [details]
Patch, v8

[Nikolas Zimmermann]

Created attachment 456902 [details]
Patch, v9

[Nikolas Zimmermann]

Depends on 237701, hence the build failures.

[Nikolas Zimmermann]

Created attachment 456979 [details]
Patch, v10

[Nikolas Zimmermann]

The mac-wk1 / mac-wk2 failures are multicol related, not affected by this patch - tested on my machine. There appears to be general flakiness with respect to multicol layout at present?

[Nikolas Zimmermann]

Committed r292690 (249482@trunk): <https://commits.webkit.org/249482@trunk>

[Fujii Hironori]

The EWS result didn't seem false positive.
Some multicol tests are ramdomly failing since 249482@trunk.
See the history of the result.
https://build.webkit.org/results/Apple-Monterey-Debug-AppleSilicon-WK2-Tests/r292690%20(1670)/results.html

[WebKit Commit Bot]

Re-opened since this is blocked by bug 239060

[Nikolas Zimmermann]

(In reply to Fujii Hironori from comment #18)
> The EWS result didn't seem false positive.
> Some multicol tests are ramdomly failing since 249482@trunk.
> See the history of the result.
> https://build.webkit.org/results/Apple-Monterey-Debug-AppleSilicon-WK2-Tests/
> r292690%20(1670)/results.html

Thanks for taking care Fujii - indeed I was misremembering two things: we do build with LBSE enabled by default on macOS, GTK & WPE -- I thought that the changes in the #if ENABLE(LBSE)... blocks are unrelated to the EWS failures :-(

And indeed I saw the flakiness before, however not on results.webkit.org, but a private dashboard ... due to my patch:

A classic one -- uninitialized variable:

diff --git a/Source/WebCore/rendering/RenderObject.cpp b/Source/WebCore/rendering/RenderObject.cpp
index 8a4d80f2b642..a65a7517b56e 100644
--- a/Source/WebCore/rendering/RenderObject.cpp
+++ b/Source/WebCore/rendering/RenderObject.cpp
@@ -2030,6 +2030,9 @@ RenderObject::RenderObjectRareData::RenderObjectRareData()
     , m_isRenderFragmentedFlow(false)
     , m_hasOutlineAutoAncestor(false)
     , m_paintContainmentApplies(false)
+#if ENABLE(LAYER_BASED_SVG_ENGINE)
+    , m_hasSVGTransform(false)
+#endif
 {
 }
 
 That solves the mystery. Will add another patch, and ask Rob for another review.
Side note: We should get rid of ADD_BOOLEAN_BITFIELD() -- the code is dangerous, we have to define the bitfields in the header, and initialize them in the source file.

That leads to bugs like this one, which can be avoided with in-place initialization... I'll note this down for a rainy afternoon.

[Nikolas Zimmermann]

Created attachment 457243 [details]
Patch, v11

[Nikolas Zimmermann]

Committed r292706 (249498@trunk): <https://commits.webkit.org/249498@trunk>