Bug 93743 – [chromium] Update HUD resources as a final step to drawing a frame

• Component: New Bugs
• Status: RESOLVED FIXED

[Dana Jansens]

[chromium] Update HUD resources as a final step to drawing a frame

[Dana Jansens]

Created attachment 157808 [details]
Patch

[Dana Jansens]

This needs tests, but I am wondering about feelings about this approach to solving http://code.google.com/p/chromium/issues/detail?id=141920

[Adrienne Walker]

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.

[Dana Jansens]

Created attachment 158616 [details]
Patch

[Dana Jansens]

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.

[Adrienne Walker]

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?

[Dana Jansens]

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.

[James Robinson]

I think it's OK for one or both LTH's to know about the HUD layer.  It's quite magical already.

[Dana Jansens]

(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?

[Adrienne Walker]

(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.

[Dana Jansens]

Created attachment 159449 [details]
Patch

[Dana Jansens]

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\!

[Adrienne Walker]

Comment on attachment 159459 [details]
Patch

R=me.

[WebKit Review Bot]

Comment on attachment 159459 [details]
Patch

Clearing flags on attachment: 159459

Committed r126046: <http://trac.webkit.org/changeset/126046>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.