793656 - Progressive tile upload causes flickering when invalidating

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Chris Lord [:cwiiis]]

If you enable progressive tile upload (via layers.progressive-paint), newly invalidated areas will flicker before being updated.

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Fix flickering during progressive tile updates

This fixes the flickering by:

1- When the resolution changes, force a single transaction
2- When updating, keep a hold of tiles within the visible region.

There was code that I think attempted this there before, but was incomplete - I've simplified it somewhat (I hope).

Comment 2

[Benoit Girard (:BenWa)]

(In reply to Chris Lord [:cwiiis] from comment #1)
> 1- When the resolution changes, force a single transaction

For this patch I think I'm ok with this (it's much better then discarding everything and having stuff appear tile by tile). In the long run I would like to improve this.

Option 1) Discard the buffer at the wrong res and draw progressively (Current) (+ No big upload jank, + show update progressively, - old content disappear and leaves a hole)
Option 2) Wait until the full paint comes in (Your patch) ( + Old content doesn't disappear and leave a hole, - upload jank, - not progressive)
Option 3) Snapshot the current tile buffer which will effectively keep the textures around. We can keep drawing the old buffer while we progressively draw the new size buffer. This means we get the best of both options while temporarily using more memory.

Comment 3

[Benoit Girard (:BenWa)]

Comment on attachment 664032 [details] [diff] [review]
Fix flickering during progressive tile updates

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

Nice, it's a big simplification of my code!

Comment 4

[Chris Lord [:cwiiis]]

(In reply to Benoit Girard (:BenWa) from comment #2)
> (In reply to Chris Lord [:cwiiis] from comment #1)
> > 1- When the resolution changes, force a single transaction
> 
> For this patch I think I'm ok with this (it's much better then discarding
> everything and having stuff appear tile by tile). In the long run I would
> like to improve this.
> 
> Option 1) Discard the buffer at the wrong res and draw progressively
> (Current) (+ No big upload jank, + show update progressively, - old content
> disappear and leaves a hole)
> Option 2) Wait until the full paint comes in (Your patch) ( + Old content
> doesn't disappear and leave a hole, - upload jank, - not progressive)
> Option 3) Snapshot the current tile buffer which will effectively keep the
> textures around. We can keep drawing the old buffer while we progressively
> draw the new size buffer. This means we get the best of both options while
> temporarily using more memory.

Plus one for Option 3 - with the possibility of doing cross-fades too :) I think Option 2 will do temporarily - shame about the jank, but we have this problem already so at least it isn't a regression.

Pushed patch to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ae26e279e17

Comment 5

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/mozilla-central/rev/1ae26e279e17