Summary: | Drawing to accelerated 2D canvas causes compositor to recompute layer tree | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Justin Novosad <junov> | ||||
Component: | New Bugs | Assignee: | Justin Novosad <junov> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | eric, jamesr, kbr, senorblanco, simon.fraser, vangelis, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Justin Novosad
2012-07-05 13:36:08 PDT
Created attachment 150979 [details]
Patch
A key piece of code that is relevant to this patch is method RenderLayer::contentChanged (not modified by this patch). This is the method that was triggering the rebuild of the compositor tree when a 2D canvas is drawn to. kbr expressed concern that we must make sure that accelerated canvas layers will still trigger the compositor in all situations with this change. I have verified that this is the case: RenderLayer::addChild will trigger the compositor (via RenderLayer::dirtyZOrderLists) when a new canvas is added. What is the code path that causes the full layer rebuild? Is it via RenderLayer::contentChanged()? (In reply to comment #4) > What is the code path that causes the full layer rebuild? Is it via RenderLayer::contentChanged()? Yes. All the methods in CanvasRenderingContext2D that draw something to the canvas end up calling CanvasRenderingContext2D::didDraw, which calls RenderBoxModelObject::contentChanged(), which calls RenderLAyer::contentChanged() But that should only cause a layer rebuild if updateLayerCompositingState() returns true. Does it? (In reply to comment #6) > But that should only cause a layer rebuild if updateLayerCompositingState() returns true. Does it? updateLayerCompositingState is returning true because RenderLayerBacking::updateGraphicsLayerConfiguration() is returning true because of this bit of code: else if (isAcceleratedCanvas(renderer)) { HTMLCanvasElement* canvas = static_cast<HTMLCanvasElement*>(renderer->node()); if (CanvasRenderingContext* context = canvas->renderingContext()) m_graphicsLayer->setContentsToCanvas(context->platformLayer()); layerConfigChanged = true; } BTW, an important benefit of this patch is that will bypass the call to updateLayerCompositingState which was a hot spot that I identified in a profiling experiment. When rendering a whole bunch of small primitives in a tight loop, all the calls to updateLayerCompositingState (can be thousands per frame) are often a much heavier burden than the compositor tree rebuild. In some experiments, I've seen up to 20% of CPU time being spent there. (In reply to comment #7) > (In reply to comment #6) > > But that should only cause a layer rebuild if updateLayerCompositingState() returns true. Does it? > > updateLayerCompositingState is returning true because RenderLayerBacking::updateGraphicsLayerConfiguration() is returning true because of this bit of code: > > else if (isAcceleratedCanvas(renderer)) { > HTMLCanvasElement* canvas = static_cast<HTMLCanvasElement*>(renderer->node()); > if (CanvasRenderingContext* context = canvas->renderingContext()) > m_graphicsLayer->setContentsToCanvas(context->platformLayer()); > layerConfigChanged = true; > } OK, for some reason Mac doesn't hit this code. I think your original patch is OK. > BTW, an important benefit of this patch is that will bypass the call to > updateLayerCompositingState which was a hot spot that I identified in a profiling experiment. When rendering a whole bunch of small primitives in a tight loop, all the calls to updateLayerCompositingState (can be thousands per frame) are often a much heavier burden than the compositor tree rebuild. In some experiments, I've seen up to 20% of CPU time being spent there. Please file a new bug on that. > > Please file a new bug on that. Done. https://bugs.webkit.org/show_bug.cgi?id=90695 Comment on attachment 150979 [details]
Patch
What is the code path that causes the full layer rebuild? Is it via RenderLayer::contentChanged()?
> What is the code path that causes the full layer rebuild? Is it via RenderLayer::contentChanged()?
Ignore that comment.
Comment on attachment 150979 [details] Patch Clearing flags on attachment: 150979 Committed r121987: <http://trac.webkit.org/changeset/121987> All reviewed patches have been landed. Closing bug. |