WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
235101
[LBSE] Introduce SVGContainerLayout
https://bugs.webkit.org/show_bug.cgi?id=235101
Summary
[LBSE] Introduce SVGContainerLayout
Nikolas Zimmermann
Reported
2022-01-12 02:26:16 PST
SVGContainerLayout contains the implementation for the two-pass layout procedure, as described in the technical design document. The container-children-layout procedure was spread over SVGRenderSupport before and is now in a central place for LBSE.
Attachments
Patch, v1
(25.74 KB, patch)
2022-01-12 05:14 PST
,
Nikolas Zimmermann
no flags
Details
Formatted Diff
Diff
Patch, v2
(25.77 KB, patch)
2022-01-14 00:41 PST
,
Nikolas Zimmermann
rbuis
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Nikolas Zimmermann
Comment 1
2022-01-12 05:14:13 PST
Created
attachment 448935
[details]
Patch, v1
Rob Buis
Comment 2
2022-01-13 01:46:11 PST
Comment on
attachment 448935
[details]
Patch, v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=448935&action=review
> Source/WebCore/ChangeLog:17 > + Currently the functionality is not observable yet, as we don't create
Probably remove the first 'yet'.
> Source/WebCore/rendering/svg/SVGContainerLayout.cpp:99 > + bool childNeededLayout = child.needsLayout();
Is it possible that childNeededLayout != needsLayout here?
> Source/WebCore/rendering/svg/SVGContainerLayout.h:44 > + static bool transformToRootChanged(const RenderObject* ancestor);
Do these 2 need to be public?
> Source/WebCore/rendering/svg/SVGContainerLayout.h:47 > + void layoutDifferentRootIfNeeded(const RenderElement&);
Different from what?
Nikolas Zimmermann
Comment 3
2022-01-14 00:41:29 PST
Created
attachment 449144
[details]
Patch, v2
Nikolas Zimmermann
Comment 4
2022-01-14 00:41:34 PST
(In reply to Rob Buis from
comment #2
)
> > Source/WebCore/ChangeLog:17 > > + Currently the functionality is not observable yet, as we don't create > > Probably remove the first 'yet'.
Fixed.
> > Source/WebCore/rendering/svg/SVGContainerLayout.cpp:99 > > + bool childNeededLayout = child.needsLayout(); > > Is it possible that childNeededLayout != needsLayout here?
Yes, there are cases where e.g. a text renderer did not originally need layout, but we force a relayout, e.g. if transform to root changes (on-screen scaling factor for text rendering -- which is an ugly topic that we should gt rid of in the future).
> > > Source/WebCore/rendering/svg/SVGContainerLayout.h:44 > > + static bool transformToRootChanged(const RenderObject* ancestor); > > Do these 2 need to be public?
Yes, they are used in other places in LBSE branch.
> > > Source/WebCore/rendering/svg/SVGContainerLayout.h:47 > > + void layoutDifferentRootIfNeeded(const RenderElement&); > > Different from what?
Different from the layout root belonging to the passed RenderElement... Used for cases like referencing a SVG filter from a different document. I agree the naming is not wise, though it's using existing terminology from SVGResources, which we do want to re-visit in future. SVGResources are mostly obsolete in LBSE, after all clipper/masker/filter... applyResource are all no-ops in LBSE as opposed to the legacy engine, since clipping/masking/filtering is handled via RenderLayer in LBSE. Anyhow, I'd like to keep the existing terminology for now, in order not to open another construction site.
Rob Buis
Comment 5
2022-01-14 01:07:06 PST
(In reply to Nikolas Zimmermann from
comment #4
)
> > Different from what? > Different from the layout root belonging to the passed RenderElement... > Used for cases like referencing a SVG filter from a different document. > > I agree the naming is not wise, though it's using existing terminology from > SVGResources, which we do want to re-visit in future. SVGResources are > mostly obsolete in LBSE, after all clipper/masker/filter... applyResource > are all no-ops in LBSE as opposed to the legacy engine, since > clipping/masking/filtering is handled via RenderLayer in LBSE.
That is great to hear!
Nikolas Zimmermann
Comment 6
2022-01-14 01:43:17 PST
Committed
r288011
(
246037@trunk
): <
https://commits.webkit.org/246037@trunk
>
Radar WebKit Bug Importer
Comment 7
2022-01-14 01:44:18 PST
<
rdar://problem/87592353
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug