Sushil Reporter
	Description • 10 years ago

+++ This bug was initially created as a clone of Bug #1040952 +++

Thebes layer is being used for Black background in Video playback. A Color layer can be used instead. Using a Thebes layer is very expensive for graphics/display pipeline as compared to a Color layer due to following reasons:
1. RGBA Thebes layer needs Gralloc buffers and it is full screen in this case. So it leads to unnecessary use of memory (think about 256 MB devices).
2. Full screen RGBA data (4 bytes per pixel) needs to be fetched by display driver for full screen Thebes layer in background. Where as Color layer can be displayed by simple SOLID_FILL operation in driver which is not expensive.
3. Unnecessary painting in framework and unnecessary data fetch in driver will impact power numbers.

Case 1: Portrait Video playback (when video controls are not visible)
2 layers are painted but not considered for composition:
E/HWComposer(  214): HWC layer[0]::ColorLayerComposite: dst=[0 36 480 800] src=[0 0 480 764] Color=ff000000 Blend=0x100
E/HWComposer(  214): HWC layer[0]::ColorLayerComposite: dst=[0 0 480 800] src=[0 0 480 800] Color=ff14120e Blend=0x100

These 2 layers are composed by HWC:
E/HWComposer(  214): HWC layer[0]::ThebesLayerComposite: dst=[0 0 480 800] src=[0 0 480 800] Transform=0 Blend=0x100
E/HWComposer(  214): HWC layer[1]::ImageLayerComposite: dst=[0 265 480 535] src=[0 0 320 180] Transform=0 Blend=0x100


Case 2: Landscape Video playback layer tree (when video controls invisible)
These 3 layers are painted but not composed. So it is unnecessary painting:
E/HWComposer(  214): HWC layer[0]::ThebesLayerComposite: dst=[0 799 480 800] src=[0 0 1 480] Transform=4 Blend=0x100
E/HWComposer(  214): HWC layer[1]::ColorLayerComposite: dst=[0 0 444 800] src=[0 0 800 444] Color=ff000000 Blend=0x100
E/HWComposer(  214): HWC layer[2]::ThebesLayerComposite: dst=[0 0 480 800] src=[0 0 800 480] Transform=4 Blend=0x105

These 2 layers are composed by HWC:
E/HWComposer(  214): HWC layer[0]::ThebesLayerComposite: dst=[0 0 480 800] src=[0 0 800 480] Transform=4 Blend=0x100
E/HWComposer(  214): HWC layer[1]::ImageLayerComposite: dst=[15 0 465 799] src=[0 0 320 180] Transform=4 Blend=0x100

	Sushil Reporter
	Comment 1 • 10 years ago

Justin,

Thanks for fixing similar bug in Camera App. This bug is for Video playback (Portrait/Landscape mode). Please check the layer tree of Video playback in v2.0 Flame.

Who can take a look at this bug? It should improve power numbers in Video playback.

	Justin D'Arcangelo [:justindarc]
	Comment 2 • 10 years ago

(In reply to Sushil from comment #1)
> Justin,
> 
> Thanks for fixing similar bug in Camera App. This bug is for Video playback
> (Portrait/Landscape mode). Please check the layer tree of Video playback in
> v2.0 Flame.
> 
> Who can take a look at this bug? It should improve power numbers in Video
> playback.

I'll take this. Thanks!

	Justin D'Arcangelo [:justindarc]
	Comment 3 • 10 years ago

Re-assigning to Hub to look at.

	Hubert Figuiere [:hub] Assignee
	Comment 4 • 10 years ago

This is the proposed change based on what was done for bug 1040952.

	Justin D'Arcangelo [:justindarc]
	Comment 5 • 10 years ago

(In reply to Hubert Figuiere [:hub] from comment #4)
> Created attachment 8478464 [details]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23280
> 
> This is the proposed change based on what was done for bug 1040952.

Hub: Can you attach layer tree dumps for before and after your patch? Thanks!

	Hubert Figuiere [:hub] Assignee
	Comment 6 • 10 years ago

I can, but I currently don't see a difference. But maybe I'm not looking in the right place.

(hence the lack of r?)

	Justin D'Arcangelo [:justindarc]
	Comment 7 • 10 years ago

Here's the layer tree on Flame (JB) during video playback on master (v2.1).

	Benoit Girard (:BenWa)
	Comment 8 • 10 years ago

The easiest way to fix this is to watch the layers dump and with app manager progressively reduce the app until the layer has the color layer you want. Then you re-enable the pieces of the app you reduced until you find what caused it.

Alternatively you can turn on display list dump (build with export B2G_DUMP_PAINTING=1), with bug 1055821 the logging will be useful in determining why you're not getting a layer.

	Hubert Figuiere [:hub] Assignee
	Comment 9 • 10 years ago

Yeah that's the conclusion we reached after our little 1:1 chat.

Thanks.

	Hubert Figuiere [:hub] Assignee
	Comment 10 • 10 years ago

I don't think we can simplify the layer tree further. I have changed where the background-color is declared in the CSS.

BEFORE

I/Gecko   ( 4903): ClientLayerManager (0xb37a0200)
I/Gecko   ( 4903):   ClientContainerLayer (0xb1c06000) [visible=< (x=0, y=0, w=854, h=555); >] [opaqueContent] [metrics={ cb=(x=0.000000, y=0.000000, w=854.000000, h=480.000000) sr=(x=0.000000, y=0.000000, w=569.333313, h=320.000000) s=(x=0.000000, y=0.000000) dp=(x=0.000000, y=0.000000, w=569.333313, h=320.000000) cdp=(x=0.000000, y=0.000000, w=569.333313, h=320.000000) scrollId=7 z=1.500 }]
I/Gecko   ( 4903):     ClientThebesLayer (0xb1c06800) [visible=< (x=0, y=0, w=854, h=480); >] [opaqueContent] [valid=< (x=0, y=0, w=854, h=480); >]
I/Gecko   ( 4903):       Info:
I/Gecko   ( 4903): 				Accumulating dp=CanvasBackgroundColor(b1c07010), f=b1e15b48 against tld=b2676780
I/Gecko   ( 4903): 				Accumulating dp=BackgroundColor(b1c07048), f=b1cda2b0 against tld=b2676780
I/Gecko   ( 4903): 				  Layer not a solid color: Can't blend colors togethers
I/Gecko   ( 4903): 				Accumulating dp=BackgroundColor(b1c070b8), f=b1cdc400 against tld=b2676780
I/Gecko   ( 4903): 				Selecting layer for tld=b2676780
I/Gecko   ( 4903): 				  Solid=0, hasImage=0, canOptimizeAwayThebes=0
I/Gecko   ( 4903): 				  Selected thebes layer=b1c06800
I/Gecko   ( 4903):     ClientContainerLayer (0xb1c0bc00) [clip=(x=0, y=0, w=854, h=480)] [transform=[ 1.1111 0; 0 1.1111; 106.75 0; ]] [visible=< (x=0, y=0, w=640, h=480); >] [preScale=0.900009, 0.900009]
I/Gecko   ( 4903):       ClientImageLayer (0xb1c0c400) [postScale=1.1111, 1.1111] [transform=[ 1.63636 0; 0 1.5; 0 0; ]] [visible=< (x=0, y=0, w=353, h=289); >] [opaqueContent]

FTER
I/Gecko   (15815): ClientLayerManager (0xb37eea00)
I/Gecko   (15815):   ClientContainerLayer (0xb3008400) [visible=< (x=0, y=0, w=854, h=555); >] [metrics={ cb=(x=0.000000, y=0.000000, w=854.000000, h=480.000000) sr=(x=0.000000, y=0.000000, w=569.333313, h=320.000000) s=(x=0.000000, y=0.000000) dp=(x=0.000000, y=0.000000, w=569.333313, h=320.000000) cdp=(x=0.000000, y=0.000000, w=569.333313, h=320.000000) scrollId=8 z=1.500 }]
I/Gecko   (15815):     ClientThebesLayer (0xb3008800) [visible=< (x=0, y=0, w=854, h=480); >] [opaqueContent] [valid=< (x=0, y=0, w=854, h=480); >]
I/Gecko   (15815):       Info:
I/Gecko   (15815): 				Accumulating dp=CanvasBackgroundColor(b2b1d410), f=b2bc3b48 against tld=b1f02400
I/Gecko   (15815): 				Accumulating dp=BackgroundColor(b2b1d448), f=b2578228 against tld=b1f02400
I/Gecko   (15815): 				Selecting layer for tld=b1f02400
I/Gecko   (15815): 				  Solid=1, hasImage=0, canOptimizeAwayThebes=0
I/Gecko   (15815): 				  Selected thebes layer=b3008800
I/Gecko   (15815):     ClientContainerLayer (0xb2b1e800) [clip=(x=0, y=0, w=854, h=480)] [transform=[ 1.1111 0; 0 1.1111; 106.75 0; ]] [visible=< (x=0, y=0, w=640, h=480); >] [preScale=0.900009, 0.900009]
I/Gecko   (15815):       ClientImageLayer (0xb2b1ec00) [postScale=1.1111, 1.1111] [transform=[ 1.63636 0; 0 1.5; 0 0; ]] [visible=< (x=0, y=0, w=353, h=289); >] [opaqueContent]

	Hubert Figuiere [:hub] Assignee
	Comment 11 • 10 years ago

BTW the output I get from Gecko is not like what :justindarc gets. But we can find the things that match though.

(this is master I use on Flame)

	Justin D'Arcangelo [:justindarc]
	Comment 12 • 10 years ago

(In reply to Hubert Figuiere [:hub] from comment #11)
> BTW the output I get from Gecko is not like what :justindarc gets. But we
> can find the things that match though.
> 
> (this is master I use on Flame)

BenWa: Do you know why Hub and I are getting different-looking layer tree dumps?

	Benoit Girard (:BenWa)
	Comment 13 • 10 years ago

Hub is showing a layer tree from a display-list dump which is starting to include debugging information about why we're picking certain layers. This is the output of a process' content to the compositor before it has been merged.

Justin is showing a compositor layer tree as we're going to render to the screen.

	bhavana bajaj [:bajaj]
	Comment 14 • 10 years ago

I originally marked this as 2.1 feature-b2g so we didn't miss this in 2.1 given the expectation we set with CAF. Discussed offline with Hema and she confirmed this would not block 2.1 FL and can be fixed after that, so hoping this is targeted in the next possible sprint after FL

	John Hu [:johnhu][:johu][:醬糊小弟]
	Comment 15 • 10 years ago

Comment on attachment 8478464 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23280

Looks good to me. Thanks. We had make the body > section to be full-size of the screen. So, the background of body > section overrides html and body's. The background of html and body are useless.

BTW, I am not really understanding why we have a Thebes layer while we set a background color. Is that because we use two containers with background color for each and gecko tries to bland those two background color? If yes, I think we can reorganize the thumbnail list to remove another background color which is used by ul.

	Hubert Figuiere [:hub] Assignee
	Comment 16 • 10 years ago

Merged
https://github.com/mozilla-b2g/gaia/commit/0d510b521679cf3019e498ff76344dcdaf3c3528

	Hubert Figuiere [:hub] Assignee
	Comment 17 • 10 years ago

If any further optimization can be found / needs to be done, see bug 1010638

The change was rolled into 2.1

	Hema Koka [:hema]
	Comment 18 • 10 years ago

Sushil/Inder,

It would be great if you could also help test the patch for 2.1

Thanks
Hema