If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Only use tiles for scrollable content

RESOLVED DUPLICATE of bug 1180326

Status

()

Core
Graphics: Layers
RESOLVED DUPLICATE of bug 1180326
3 years ago
2 years ago

People

(Reporter: nical, Unassigned)

Tracking

(Depends on: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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.
(Reporter)

Comment 1

3 years ago
Created attachment 8537300 [details] [diff] [review]
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.
Attachment #8537300 - Flags: review?(bgirard)
We're saving memory on B2G, I'd personally prefer a single code path on desktop...
Comment hidden (obsolete)
Comment hidden (obsolete)
Disregard my comments, I assumed you were posting another patch.
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.
Attachment #8537300 - Flags: review+
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.
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.
Assignee: nical.bugzilla → nobody
(Reporter)

Comment 9

3 years ago
(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).
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.
Depends on: 1126950
(Reporter)

Comment 11

3 years ago
(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)
(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.
Blocks: 1130545
Blocks: 1154834
Blocks: 1157465
This was implemented in part 6 in bug 1180326.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1180326
You need to log in before you can comment on or make changes to this bug.