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.
Created attachment 448935 [details] Patch, v1
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?
Created attachment 449144 [details] Patch, v2
(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.
(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!
Committed r288011 (246037@trunk): <https://commits.webkit.org/246037@trunk>
<rdar://problem/87592353>