Bug 216677 – REGRESSION(r267137): PaintFrequencyTracker needs to track all the painting

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Said Abou-Hallawa]

r267137 made PaintFrequencyTracker track only the painting which comes through Page::updateRendering(). This caused ~30% regression in the MotionMark Design test because it enlarged the gap between the paintings. So PaintFrequencyTracker will never encounter a PaintFrequency::High. This means we lost the optimization that could be gained from replaying back the DisplayList of the text runs.

[Said Abou-Hallawa]

<rdar://problem/69080461>

[Said Abou-Hallawa]

Created attachment 409098 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 409098 [details]
Patch

What painting does not "come through" Page::updateRendering() ?

[Said Abou-Hallawa]

Updating control tints: See the call stack in https://bugs.webkit.org/show_bug.cgi?id=216190#c10.

Or WebPage snapshotting:

WebCore: WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
WebCore: WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
WebCore: WebCore::RenderLayer::paintLayerWithEffects(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
WebCore: WebCore::RenderLayer::paintLayer(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
WebCore: WebCore::RenderLayer::paintList(WebCore::RenderLayer::LayerList, WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
WebCore: WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
WebCore: WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
WebCore: WebCore::RenderLayer::paintLayerWithEffects(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
WebCore: WebCore::RenderLayer::paintLayer(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
WebCore: WebCore::RenderLayer::paint(WebCore::GraphicsContext&, WebCore::LayoutRect const&, WebCore::LayoutSize const&, WTF::OptionSet<WebCore::PaintBehavior>, WebCore::RenderObject*, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>, WebCore::RenderLayer::SecurityOriginPaintPolicy, WebCore::EventRegionContext*)
WebCore: WebCore::FrameView::paintContents(WebCore::GraphicsContext&, WebCore::IntRect const&, WebCore::Widget::SecurityOriginPaintPolicy, WebCore::EventRegionContext*)
WebCore: WebCore::FrameView::paintContentsForSnapshot(WebCore::GraphicsContext&, WebCore::IntRect const&, WebCore::FrameView::SelectionInSnapshot, WebCore::FrameView::CoordinateSpaceForSnapshot)
WebKit: WebKit::WebPage::paintSnapshotAtSize(WebCore::IntRect const&, WebCore::IntSize const&, unsigned int, WebCore::Frame&, WebCore::FrameView&, WebCore::GraphicsContext&)
WebKit: WebKit::WebPage::snapshotAtSize(WebCore::IntRect const&, WebCore::IntSize const&, unsigned int)
WebKit: WebKit::WebPage::scaledSnapshotWithOptions(WebCore::IntRect const&, double, unsigned int)

[Simon Fraser (smfr)]

But none of those code paths should be performance-sensitive. updateControlTints shouldn't even paint glyph buffers.

[Said Abou-Hallawa]

#2	WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)
#3	WebCore::RenderLayerBacking::paintIntoLayer(WebCore::GraphicsLayer const*, WebCore::GraphicsContext&, WebCore::IntRect const&, WTF::OptionSet<WebCore::PaintBehavior>, WebCore::EventRegionContext*)::$_22::operator()(WebCore::RenderLayer&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>) const
#4	WebCore::RenderLayerBacking::paintIntoLayer(WebCore::GraphicsLayer const*, WebCore::GraphicsContext&, WebCore::IntRect const&, WTF::OptionSet<WebCore::PaintBehavior>, WebCore::EventRegionContext*)
#5	WebCore::RenderLayerBacking::paintContents(WebCore::GraphicsLayer const*, WebCore::GraphicsContext&, WebCore::FloatRect const&, unsigned int)
#6	WebCore::GraphicsLayer::paintGraphicsLayerContents(WebCore::GraphicsContext&, WebCore::FloatRect const&, unsigned int)
#7	WebCore::GraphicsLayerCA::platformCALayerPaintContents(WebCore::PlatformCALayer*, WebCore::GraphicsContext&, WebCore::FloatRect const&, unsigned int)
#8	WebCore::PlatformCALayer::drawLayerContents(WebCore::GraphicsContext&, WebCore::PlatformCALayer*, WTF::Vector<WebCore::FloatRect, 5ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, unsigned int)
#9	WebCore::TileGrid::platformCALayerPaintContents(WebCore::PlatformCALayer*, WebCore::GraphicsContext&, WebCore::FloatRect const&, unsigned int)
#10	[WebSimpleLayer drawInContext:]

[Said Abou-Hallawa]

I was wrong. I understand now what is happening.

Resetting the Page::m_renderingUpdateTimestamp at the end of Page::updateRendering() is wrong since this will make RenderLayer::paintLayerContents() sees it always zero. There is no direct call from Page::updateRendering() to RenderLayer::paintLayerContents(). Actual painting is scheduled in next run loop through TiledCoreAnimationDrawingArea::scheduleRenderingUpdateRunLoopObserver(). So PaintFrequencyTracker does not see the gap between the painting increased as I stated above. In fact PaintFrequencyTracker is not tracking any painting because the timestamp is always zero. And this is why we were falling always to the none DisplayList replaying back scenario.

[Simon Fraser (smfr)]

That makes more sense. Be sure to check iOS and macOS which paint differently.

[Said Abou-Hallawa]

I made Page::updateRendering() records the last MonotonicTime::now() when it called and make PaintFrequencyTracker use this timestamp. This is the same as the change in r266677. The only difference is PaintFrequencyTracker will track all the painting from now on. When Page::lastRenderingUpdateTimestamp() is zero, PaintFrequencyTracker will use call MonotonicTime::now() which will happen only in cases like calling updateControlTints() before calling Page::updateRendering() is called.

[Said Abou-Hallawa]

Created attachment 409149 [details]
Patch

[EWS]

Committed r267254: <https://trac.webkit.org/changeset/267254>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409149 [details].