1112193 - Only use tiles for scrollable content

Status: RESOLVED DUPLICATE of bug 1180326

(Core :: Graphics: Layers, defect)

Description

[Nicolas Silva [:nical]]

This is already what we do on b2g. There is no reason to do it differently on android at least, and I can't think of anything that would make it a bad thing on desktop.

On the other hand, it is very common to have layers that are smaller in at least one dimension than the tile size (like the chrome and small things that are animated), so we'd save some memory from not using tiling there.

Comment 1

[Nicolas Silva [:nical]]

Attachment: Use the same logic to enable tiles on all pplatform

The patch is quite simple, so just throwing it out there. We may decide we don't want it (yet), although it would raise the question of why we do it on b2g.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

We're saving memory on B2G, I'd personally prefer a single code path on desktop...

Comment 3

[Benoit Girard (:BenWa)]

I believe part of the reason is because hardware composer has a limit on the number of draw calls. Or at least did. So either:
1) HWC on hardware we target no longer has that limitation.
2) We use an alternative way of getting HWC on the pages we care about like fullscreen video playback.

Comment 4

[Benoit Girard (:BenWa)]

Comment on attachment 8537300 [details] [diff] [review]
Use the same logic to enable tiles on all pplatform

Taking off the review until we can prove that this wouldn't regress HWC on devices we care about.

Comment 5

[Benoit Girard (:BenWa)]

Disregard my comments, I assumed you were posting another patch.

Comment 6

[Benoit Girard (:BenWa)]

Comment on attachment 8537300 [details] [diff] [review]
Use the same logic to enable tiles on all pplatform

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

The only drawback I can see is that if we have non scrollable layers that need to resize this might be a bit worse. But I expect overall things will be better with this patch.

Comment 7

[Matt Woodrow (:mattwoodrow)]

This is going to make us start using ContentClientIncremental again on OSX, which we were hoping to be able to delete.

We could use ContentClient{Single|Double}Buffered, but that would require sync transactions, which I doubt we want either.

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

This has gotten a life of it's own too quickly :)  Nical, let's have an off line conversation about why we would not have tiling everywhere all the time, in the meantime we do not want to take a step back where we start using tiling less.  Please don't spend time on it and no landing things until we clear this up.

Comment 9

[Nicolas Silva [:nical]]

(In reply to Milan Sreckovic [:milan] from comment #8)
> This has gotten a life of it's own too quickly :)  Nical, let's have an off
> line conversation about why we would not have tiling everywhere all the
> time, in the meantime we do not want to take a step back where we start
> using tiling less.  Please don't spend time on it and no landing things
> until we clear this up.

Yeah, we'll talk about it next tuesday. The patch is already there and green on try, so it's only a question of whether we want it or not (which is a not as simple a question as I originally thought).

Comment 10

[Jeff Muizelaar [:jrmuizel]]

We had a bit of a discussion about this in the Toronto office.
- We all agreed in principle that it would probably be better to not use tiles for non-scrollable content (especially if layout can do more work to make sure that the size of the layers is not changing much).
- We also agreed that unifying the B2G and OS X approaches is a good thing
- Currently, non-tiled content requires sync transactions. If we do switch to a separate path for non-scrollable content on OS X, it should not use sync transactions.

Comment 11

[Nicolas Silva [:nical]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #10)
> We had a bit of a discussion about this in the Toronto office.
> - We all agreed in principle that it would probably be better to not use
> tiles for non-scrollable content (especially if layout can do more work to
> make sure that the size of the layers is not changing much).
> - We also agreed that unifying the B2G and OS X approaches is a good thing
> - Currently, non-tiled content requires sync transactions. If we do switch
> to a separate path for non-scrollable content on OS X, it should not use
> sync transactions.

Sweet!

We could add a very simple non-tiled-non-rotating ContentClient which would look like updating a tiled layer but with only one tile (using a similar copy-on-write trick)

Comment 12

[Jeff Muizelaar [:jrmuizel]]

(In reply to Nicolas Silva [:nical] from comment #11)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #10)
> > We had a bit of a discussion about this in the Toronto office.
> > - We all agreed in principle that it would probably be better to not use
> > tiles for non-scrollable content (especially if layout can do more work to
> > make sure that the size of the layers is not changing much).
> > - We also agreed that unifying the B2G and OS X approaches is a good thing
> > - Currently, non-tiled content requires sync transactions. If we do switch
> > to a separate path for non-scrollable content on OS X, it should not use
> > sync transactions.
> 
> Sweet!
> 
> We could add a very simple non-tiled-non-rotating ContentClient which would
> look like updating a tiled layer but with only one tile (using a similar
> copy-on-write trick)

This seems like the best plan.

Comment 13

[Markus Stange [:mstange]]

This was implemented in part 6 in bug 1180326.