[chromium] Update HUD resources as a final step to drawing a frame
Created attachment 157808 [details] Patch
This needs tests, but I am wondering about feelings about this approach to solving http://code.google.com/p/chromium/issues/detail?id=141920
Comment on attachment 157808 [details] Patch I think I'd really prefer not to add some step that applies to all layers. Updating resources that you have already put in quads also seems potentially fragile. It may work now, but it seems easy to break, and we have no HUD tests. I think the only information that you don't have immediately post-calcDraw is the screen space occlusion rects. My thought is that you can save all but those rects explicitly on the HUD layer right after calcDraw and then make the HUD layer the container for the screen space occlusion rects to get those right when it needs it during appendQuads since it goes last.
Created attachment 158616 [details] Patch
Here's a new proposal for ya! 1. Have the HUD layer save a pointer to itself on commit in the CCLTHI. 2. CCLTHI clears this pointer each commit to avoid staleness. 3. CCLTHI calls the HUD layer to update its texture just before it draws the frame.
Comment on attachment 158616 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158616&action=review > Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplayLayerImpl.cpp:77 > + hostImpl->setHudLayer(this); Can you move this to CCLayerTreeHost::finishCommitOnImplThread with the rest of the syncing code, clearing the hud layer pointer if it doesn't exist?
Comment on attachment 158616 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158616&action=review >> Source/WebCore/platform/graphics/chromium/cc/CCHeadsUpDisplayLayerImpl.cpp:77 >> + hostImpl->setHudLayer(this); > > Can you move this to CCLayerTreeHost::finishCommitOnImplThread with the rest of the syncing code, clearing the hud layer pointer if it doesn't exist? The thing is CCLTH doesn't know the impl pointer unless we store it in the HUDLayer over there. This lets us avoid storing the Impl pointer on the non-impl tree anywhere.
I think it's OK for one or both LTH's to know about the HUD layer. It's quite magical already.
(In reply to comment #8) > I think it's OK for one or both LTH's to know about the HUD layer. It's quite magical already. Yeh, as it does in this patch. But I mean there's no link from HUDLayerChromium to CCHUDLayerImpl unless we store the impl layer's pointer in the HUDLayerChromium when we create the impl layer. Then the CCLTH could pull it out that way. But it seems more sketchy to me to store a pointer from Layer->LayerImpl as that's something we don't do ever now. Or we do something in TreeSychronizer, or do a tree walk to find the CCHUDLayerImpl by ID in CCLTH. Preferences?
(In reply to comment #9) > (In reply to comment #8) > > I think it's OK for one or both LTH's to know about the HUD layer. It's quite magical already. > > Yeh, as it does in this patch. But I mean there's no link from HUDLayerChromium to CCHUDLayerImpl unless we store the impl layer's pointer in the HUDLayerChromium when we create the impl layer. Then the CCLTH could pull it out that way. > > But it seems more sketchy to me to store a pointer from Layer->LayerImpl as that's something we don't do ever now. > > Or we do something in TreeSychronizer, or do a tree walk to find the CCHUDLayerImpl by ID in CCLTH. > > Preferences? A tree walk to find the layer by ID during commit seems reasonable. I'd rather not complicate TreeSynchronizer anymore.
Created attachment 159449 [details] Patch
Created attachment 159459 [details] Patch Oh, CC header paths have changed. I made a layer tree walk in CCLTH to push over the hud layer pointer as requested\!
Comment on attachment 159459 [details] Patch R=me.
Comment on attachment 159459 [details] Patch Clearing flags on attachment: 159459 Committed r126046: <http://trac.webkit.org/changeset/126046>
All reviewed patches have been landed. Closing bug.