Add a mechanism to dump the PlatformCALayer subtree of a GraphicsLayer, for testing
Created attachment 426083 [details] Patch
Chances are high that there are iOS rebaselines needed, I haven't run the tests on iOS yet.
Created attachment 426092 [details] Patch
Yay!!
Comment on attachment 426092 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426092&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:4040 > + UNUSED_PARAM(flags); hmm? > Source/WebCore/platform/graphics/ca/PlatformCALayer.h:124 > + virtual PlatformCALayerList sublayersForLogging() = 0; const? > Source/WebCore/rendering/RenderLayerCompositor.cpp:2276 > + frameView.updateLayoutAndStyleIfNeededRecursive(); I feel like there were tests where we explicitly didn't want to update layout here.
(In reply to Simon Fraser (smfr) from comment #5) > Comment on attachment 426092 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426092&action=review > > > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:4040 > > + UNUSED_PARAM(flags); > > hmm? Whoops. > > Source/WebCore/platform/graphics/ca/PlatformCALayer.h:124 > > + virtual PlatformCALayerList sublayersForLogging() = 0; > > const? Sure. > > Source/WebCore/rendering/RenderLayerCompositor.cpp:2276 > > + frameView.updateLayoutAndStyleIfNeededRecursive(); > > I feel like there were tests where we explicitly didn't want to update > layout here. The internals caller did it before, so I don't know how that can be.
(In reply to Tim Horton from comment #6) > (In reply to Simon Fraser (smfr) from comment #5) > > Comment on attachment 426092 [details] > > Patch > > > > > Source/WebCore/rendering/RenderLayerCompositor.cpp:2276 > > > + frameView.updateLayoutAndStyleIfNeededRecursive(); > > > > I feel like there were tests where we explicitly didn't want to update > > layout here. > > The internals caller did it before, so I don't know how that can be. Frame did it too :) so many layouts
Created attachment 426131 [details] Patch
Comment on attachment 426131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426131&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3967 > + ts << indent << "(bounds " << layer->bounds().width() << " " << layer->bounds().height() << ")\n"; size rather than bounds? > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3972 > + if (!flags.contains(PlatformLayerTreeAsTextFlags::IgnoreChildren)) { Double negative is a bit hard to read. Maybe the flag should be IncludeChildren?
(In reply to Simon Fraser (smfr) from comment #9) > Comment on attachment 426131 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426131&action=review > > > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3967 > > + ts << indent << "(bounds " << layer->bounds().width() << " " << layer->bounds().height() << ")\n"; > > size rather than bounds? I'm going to leave this be because it matches GraphicsLayer. > > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:3972 > > + if (!flags.contains(PlatformLayerTreeAsTextFlags::IgnoreChildren)) { > > Double negative is a bit hard to read. Maybe the flag should be > IncludeChildren? This too, mostly because I feel like the double negative is worth not having a flag set by default.
Committed r276085 (236597@main): <https://commits.webkit.org/236597@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426131 [details].
rdar://76729543