1199371 - Don't create TextureClients for video when the video isn't visible

Status: RESOLVED WONTFIX

(Core :: Graphics: Layers, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: image-bridge-idle

Playing youtube videos in background tabs spends a lot of CPU time uploading to the GPU.

Skipping this seems like the right thing to do.

Comment 1

[Nicolas Silva [:nical]]

Comment on attachment 8653623 [details] [diff] [review]
image-bridge-idle

Review of attachment 8653623 [details] [diff] [review]:
-----------------------------------------------------------------

A comment explaining that we keep track of whether the ImageContainer is "attached" to avoid sending TextureClients that won't be displayed would be very nice.

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/446055bf8ee4

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aef5095f71da

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/446055bf8ee4
https://hg.mozilla.org/mozilla-central/rev/aef5095f71da

Comment 5

[Carsten Book [:Tomcat]]

sorry had to back this out since this might have made OS X reftest failung more frequently in https://treeherder.mozilla.org/logviewer.html#?job_id=13639839&repo=mozilla-inbound

Comment 6

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db9511cfd79d
https://hg.mozilla.org/integration/mozilla-inbound/rev/76d35d4fd847

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Image Bridge Idle

We need to synchronously send the message to the image bridge thread, otherwise we have a race condition where the main thread can schedule a composite before the ImageContainer gets updated.

Comment 8

[Nicolas Silva [:nical]]

Comment on attachment 8659919 [details] [diff] [review]
Image Bridge Idle

Review of attachment 8659919 [details] [diff] [review]:
-----------------------------------------------------------------

That's a bit sad (having to rely on more synchronous communication). Can't we do this only for stuff that comes from a main-thread transaction ? we have ImageContainer::SetCurrentImageInTransaction.

Comment 9

[Nicolas Silva [:nical]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #7)
> We need to synchronously send the message to the image bridge thread,
> otherwise we have a race condition where the main thread can schedule a
> composite before the ImageContainer gets updated.

Isn't this in fact a problem with the test rather than gecko ? ImageBridge is for things which don't require to be in sync with layer transactions. If we want to make the test work we should fix the test rather than add synchronous roundtrips on gecko's busiest thread.

Comment 10

[Nicolas Silva [:nical]]

Comment on attachment 8659919 [details] [diff] [review]
Image Bridge Idle

Review of attachment 8659919 [details] [diff] [review]:
-----------------------------------------------------------------

Scratch my last comment, I misunderstood the a crucial part of the patch in my review and Matt made a valid point about the first paint being synchronous so that there is content in the first composition after an image layer has been made visible.

Comment 11

[u279076]

Matt, did the patch in comment 10 ever get landed?

Comment 12

[Matt Woodrow (:mattwoodrow)]

Nope. I don't think we need it any more though since we're working on disabling the decoder for videos that aren't visible.