Open Bug 1467151 Opened 2 years ago Updated 4 months ago

Enable OMTP on Android

Categories

(Core :: Graphics: Layers, task, P3)

Unspecified
Android
task

Tracking

()

Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- affected

People

(Reporter: rhunt, Assigned: rhunt, NeedInfo)

References

(Blocks 1 open bug)

Details

(Whiteboard: [gfx-noted][geckoview:p2])

Attachments

(1 file)

Enabling OMTP on android should run into the same freetype issue we are running into on bug 1432531.

With that resolved I think there are two other changes we need to get OMTP working on android.

1. Audit layers.progressive-paint to see if it is safe
2. Port layers.tiles.edge-padding to work on the paint thread

I believe these both should be fairly simple.
Depends on: 1471261
Depends on: 1471639
Here's the latest try run with the patches from bug 1471639 [1]. I'll probably land the patches together when the time comes.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=4564d088c2f558505b055dd3859cb7f01080bc18
Comment on attachment 8992771 [details]
Bug 1467151 - Enable OMTP on all remaining platforms.

https://reviewboard.mozilla.org/r/257618/#review264626
Attachment #8992771 - Flags: review?(nical.bugzilla) → review+
(In reply to Ryan Hunt [:rhunt] from comment #0)
> Enabling OMTP on android should run into the same freetype issue we are
> running into on bug 1432531.
> 
> With that resolved I think there are two other changes we need to get OMTP
> working on android.
> 
> 1. Audit layers.progressive-paint to see if it is safe
> 2. Port layers.tiles.edge-padding to work on the paint thread
> 
> I believe these both should be fairly simple.

I forgot to write up about (1).

The idea behind progressive painting painting, was to paint only the user visible
parts of the display port first, then paint more tiles outside of it, cancelling
the paints if the user started scrolling away from those tiles. It was hoped to
reduce checkerboarding and is only enabled on android.

The important thing about progressive painting from the perspective of OMTP is that
it works by scheduling a repeat transaction to paint extra content. This is safe
because we don't special case this transaction and it gets flushed like normal.

A problem with this though, is that it sounds like each transaction in a progressive
paint will usually only paint one tile. So we get the overhead of OMTP, with
immediate blocking at the beginning of the next repeat transaction, and without
any parallel painting. If we are only ever painting one tile at a time, there
isn't any benefit.

From an irc conversation, kats thinks that it might be worth it to disable
progressive painting if we were to turn on parallel painting. Longer term we
could also get progressive painting working better with the paint thread.

The important thing if we were to switch from progressive painting to OMTP is
to make sure we don't regress anything. We do have checkerboarding telemetry,
and kats says we do get a decent amount of submissions on it, so we could
detect a regression there. Possibly also on talos or user comments.

I'm not really sure who's call this is. Snorp, do you have any thoughts on trying
this on nightly and seeing if there are any regressions?
Flags: needinfo?(snorp)
(In reply to Ryan Hunt [:rhunt] from comment #4)
> (In reply to Ryan Hunt [:rhunt] from comment #0)
> > Enabling OMTP on android should run into the same freetype issue we are
> > running into on bug 1432531.
> > 
> > With that resolved I think there are two other changes we need to get OMTP
> > working on android.
> > 
> > 1. Audit layers.progressive-paint to see if it is safe
> > 2. Port layers.tiles.edge-padding to work on the paint thread
> > 
> > I believe these both should be fairly simple.
> 
> I forgot to write up about (1).
> 
> The idea behind progressive painting painting, was to paint only the user
> visible
> parts of the display port first, then paint more tiles outside of it,
> cancelling
> the paints if the user started scrolling away from those tiles. It was hoped
> to
> reduce checkerboarding and is only enabled on android.
> 
> The important thing about progressive painting from the perspective of OMTP
> is that
> it works by scheduling a repeat transaction to paint extra content. This is
> safe
> because we don't special case this transaction and it gets flushed like
> normal.
> 
> A problem with this though, is that it sounds like each transaction in a
> progressive
> paint will usually only paint one tile. So we get the overhead of OMTP, with
> immediate blocking at the beginning of the next repeat transaction, and
> without
> any parallel painting. If we are only ever painting one tile at a time, there
> isn't any benefit.
> 
> From an irc conversation, kats thinks that it might be worth it to disable
> progressive painting if we were to turn on parallel painting. Longer term we
> could also get progressive painting working better with the paint thread.
> 
> The important thing if we were to switch from progressive painting to OMTP is
> to make sure we don't regress anything. We do have checkerboarding telemetry,
> and kats says we do get a decent amount of submissions on it, so we could
> detect a regression there. Possibly also on talos or user comments.
> 
> I'm not really sure who's call this is. Snorp, do you have any thoughts on
> trying
> this on nightly and seeing if there are any regressions?

I should add that Jamie just piped in on irc that it might be 2-4 tiles (which
is a row on the screen). Which might change things slightly.
(In reply to Ryan Hunt [:rhunt] from comment #5)
> I should add that Jamie just piped in on irc that it might be 2-4 tiles
> (which
> is a row on the screen). Which might change things slightly.

I suppose another option then would be to enable OMTP on android with progressive
painting and see if there are any regressions.

That wouldn't catch if we're just staying the same and could get better from
disabling progressive painting, though.
Kevin if you get a chance we're curious about how scrolling feels with:

1. OMTP on and progressive painting on
2. OMTP on and progressive painting off

(compared with how things are now, OMTP off, progressive painting on)

Not sure how best to test this... but smoke testing could be interesting.

I think the config options are:

OMTP on: layers.omtp.enabled = true
Progressive painting off: layers.low-precision-buffer = false, and layers.progressive-paint = false

Ryan (and maybe Randall Barker) are good contact points for more details.
Flags: needinfo?(kbrosnan)
Ioana, Kevin is out, is comment 7 something you could help with?
Flags: needinfo?(kbrosnan) → needinfo?(ioana.chiorean)
Re comment 7

Just my 2c worth, I much prefer progressive painting ON.

When progressive painting is OFF, a fast fling can result in a moment of blank screen. With progressive painting ON, I get to see the pixellated version of the page during the same fling, which I much prefer because I never see a blank white screen.

This is particularly noticeable on a slow entry level phone (Snapdragon 412 / 1.4 GHz ARM53).

For example:-

www.bikeradar.com:- load the page; wait a second or so.  Do a big strong fling. Wait for it to stop. Fling in the opposite direction. Repeat the opposing flings. With progressive paint OFF there is a moment of white screen on each fling. With progressive paint ON there is a moment of pixellated screen content which IMHO is much more elegant than the white screen.

I turned progressive painting off using layers.low-precision-buffer = false, and layers.progressive-paint = false.
Actually I think what I am focusing on is the low precision buffer part rather than the progressive paint part. I assume the pixellated image during scrolling is the low precision buffer. Are low precision buffer & progressive paint interlinked so strongly?
OS: Unspecified → Android
Summary: Enable OMTP on android → Enable OMTP on Android
Whiteboard: [gfx-noted] → [gfx-noted][geckoview]
Yes, the behaviour you're describing is the low precision buffer part of things. It should be possible to disable progressive paint without affecting the low precision buffer. I think it's harder to do the opposite (disable the low-precision buffer without affecting progressive paint).
thanks

So I prefer low precision buffer on, its presence nicely disguises some tile painting during crazy fast flings.

I might have a look at progressive paint on/off while keeping low precision buffer on. I'll drop a comment if I see anything of note with my Mk I eyeball.

cheers
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11)
> Yes, the behaviour you're describing is the low precision buffer part of
> things. It should be possible to disable progressive paint without affecting
> the low precision buffer. I think it's harder to do the opposite (disable
> the low-precision buffer without affecting progressive paint).

Yes they can be disabled separately, but it turns out they both use repeat
transactions. [1] So we'd run into the same sync flush issue with low precision
enabled.

It does seem to me though that we could just remove that 'if' and allow the
updates in the same transaction. I'm not sure if there is some code that
relies on them being separate though.

[1] https://searchfox.org/mozilla-central/rev/ad36eff63e208b37bc9441b91b7cea7291d82890/gfx/layers/client/ClientTiledPaintedLayer.cpp#577
OK I am only a humble user & feel I'm taking up a lot of your time.

I think keeping low precision buffer is essential and maybe the flushage of which you speak is something worth living with.

Particularly true with long Wikipedia articles, there is bug 1334990 and the low precision buffer helps disguise the slow painting. A bit. Particular important on a slow phone.

So I would suggest not fiddling with low precision buffering at least until bug 1334990 is fixed.

cheers
Whiteboard: [gfx-noted][geckoview] → [gfx-noted][geckoview:p2]
I'm pretty late here but I'm ok with turning it on in Nightly. I'd prefer if we have some telemetry to tell us if it improved things or not, though.
Flags: needinfo?(snorp)
FWIW I have been using OMTP for the last couple of weeks. I've noticed no adverse effects. On my very slow ARM53 1.4 GHz phone I think I see a tiny improvement in scroll smoothness... Maybe. On my faster ARM73 2 GHz, no real difference.
Duplicate of this bug: 1509530

James, any new thoughts on this? What's the latest here? Does this make a difference in the current GV/Fenix context?

Flags: needinfo?(snorp)
Type: defect → task

I don't really have any idea what the status is here. Jessie, has anyone looked at this in gfx recently?

Flags: needinfo?(snorp) → needinfo?(jbonisteel)

No, we have not. Not likely to be something we spend time on before we start shipping WebRender on Android.

Flags: needinfo?(jbonisteel)
You need to log in before you can comment on or make changes to this bug.