We can easily bring in text support from legacy -> LBSE. No need to split up RenderSVGText in LegacyRenderSVGText / RenderSVGText (LBSE): the inheritance structure hasn't changed -- RenderSVGText always already inherited from RenderLayerModelObject in the legacy engine, it just force-disabled layer creation. Change that.
Created attachment 458312 [details] Patch, v1 (depends on bug 239717)
Created attachment 458313 [details] Patch, v1 (includes patch from bug 239717)
Want to get some EWS results overnight... let's hope I got it right ;-)
Created attachment 458315 [details] Patch, v2 (includes patch from bug 239717)
Created attachment 458345 [details] Patch, v3 (depends on bug 239717)
Created attachment 458346 [details] Patch, v3 (includes patch from bug 239717)
Created attachment 458469 [details] Patch, v4
<rdar://problem/92637427>
Comment on attachment 458469 [details] Patch, v4 View in context: https://bugs.webkit.org/attachment.cgi?id=458469&action=review > Source/WebCore/rendering/RenderObject.cpp:527 > +#endif Why is this needed and why now? > Source/WebCore/rendering/svg/RenderSVGBlock.cpp:51 > +#endif Should this have been introduced in an earlier patch? What is the relation to RenderSVGText? > Source/WebCore/rendering/svg/RenderSVGBlock.cpp:129 > + if (style().visibility() != Visibility::Visible && !enclosingLayer()->hasVisibleContent()) Is the second condition not enough?
Comment on attachment 458469 [details] Patch, v4 View in context: https://bugs.webkit.org/attachment.cgi?id=458469&action=review >> Source/WebCore/rendering/RenderObject.cpp:527 >> +#endif > > Why is this needed and why now? Now that <text> is enabled the code path is hit for the first time in LBSE. We'll assert without it in a few tests. We currently disallow partial layouts, no SVG renderer can be a relayout boundary. A container position/size changes affects its parent, due to the way SVG containers are constructed. >> Source/WebCore/rendering/svg/RenderSVGBlock.cpp:51 >> +#endif > > Should this have been introduced in an earlier patch? What is the relation to RenderSVGText? No, it's correct to introduce it here: Every transformable SVG renderer has to take care of updating these flags (in LBSE-only). Therefore renderers such as RenderSVGBlock/Text that become LBSE-aware, now need this logic. >> Source/WebCore/rendering/svg/RenderSVGBlock.cpp:129 >> + if (style().visibility() != Visibility::Visible && !enclosingLayer()->hasVisibleContent()) > > Is the second condition not enough? No, it's not. The code here was originally based on RenderBox::clippedOverflowRect() ~ last rebased and compared in Oct 2021. Today, RenderBox::clippedOverflowRect() looks different: LayoutRect RenderBox::clippedOverflowRect(const RenderLayerModelObject* repaintContainer, VisibleRectContext context) const { if (isInsideEntirelyHiddenLayer()) return { }; ... To recap: RenderLayer queries the renderers clippedOverflowRectForRepaint() (== clippedOverflowRect(), with a predefined VisibleRectContext) to determine the repaint rect for the layer, in RenderLayer::computeRepaintRects(). The two checks "style().visibility() != Visibility::Visible" and "!enclosingLayer()->hasVisibleContent()" were encapsulated in a new 'isInsideEntirelyHiddenLayer()' helper recently introduced by Simon (https://bugs.webkit.org/show_bug.cgi?id=235242). The style().visibility() check only considers the current renderer, whereas the enclosingLayer()->hasVisibleContent() in principle depends on the visibility status of all children of the enclosing layer. An example to clarify this: 1) if 'visibility' is set to 'visible' the two checks are actually equivalent, RenderLayer::computeHasVisibleContent() returns true if 'visibility' is set to 'visible'. 2) If 'visibility' is set to 'hidden', we still might need to return a non-empty 'visual overflow rect', if the layers' renderer has children with 'visibility: visible' that themselves have no own layer. I will adapt the code.
Created attachment 459413 [details] Patch, v5
Comment on attachment 459413 [details] Patch, v5 Looks good, just the ChangeLog will have to be removed before landing.
Pull request: https://github.com/WebKit/WebKit/pull/687
Committed r294385 (250682@main): <https://commits.webkit.org/250682@main> Reviewed commits have been landed. Closing PR #687 and removing active labels.