Panning and pinch zooming causes page to whiteout

RESOLVED FIXED in Firefox 30

Status

()

defect
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: zao, Assigned: cwiiis)

Tracking

({reproducible})

Trunk
Firefox 31
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox29 unaffected, firefox30+ fixed, firefox31 verified, b2g-v1.4 fixed, b2g-v2.0 fixed, fennec30+)

Details

()

Attachments

(6 attachments, 1 obsolete attachment)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140312030201

Steps to reproduce:

Navigate to http://umumat.se
Pan towards the right side of the page
Pan around in general.


Actual results:

In certain page positions the page shows regions of white on the right and bottom, flickering in and out of existence as you move. It stays stable good/broken when not moving and repeats as you pass over the same locations.


Expected results:

No white position-dependent regions of white.
OS: Linux → Android
Hardware: x86_64 → ARM
This is a Sony Ericson Xperia V (LT25i) running the recent Android 4.3, w/ Telia as telco.
I do indeed see this after some panning and pinch-zooming on the content in the URL, content will essentially whiteout.

Screenshot from my Galaxy SIV (Android 4.4.2)

When this happens I do indeed see the typical 'Aborting draw due to resolution change' messages flood console. E.g, 

D/GeckoLayerClient( 3707): Aborting draw due to resolution change: 2.5859103 != 2.587752

CC'ing some graphics folks
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: reproducible
Summary: panning causes page to flicker white → Panning and pinch zooming causes page to whiteout
See Also: → 982651
Chris, can you look into this? Looks like a tiling/progressive update bug.
Flags: needinfo?(chrislord.net)
Yup, might as well knock all of these on the head at once...
Assignee: nobody → chrislord.net
Status: NEW → ASSIGNED
Flags: needinfo?(chrislord.net)
It seems that the 'cancel if zoom has changed' behaviour causes this, but I've not looked into why that is yet. Will take a look after bug 83208, which unfortunately, does not fix this (at least, it doesn't seem that way at the moment).

Possibly there a situation that cancelling is affecting the valid region incorrectly somehow, which would also explain why there are sometimes odd partially-black/coloured tiles in the low precision buffer.
tracking-fennec: --- → ?
The main error previously was to use the valid region of the layer instead of the valid region of the tiled buffers, which more accurately represents what's been drawn when using progressive rendering.

This doesn't solve all the issues, there are still odd single-coloured blocks in the low precision buffer, and I've not managed to track down why that is yet. This makes things quite a bit better at least.
Attachment #8397170 - Flags: review?(bugmail.mozilla)
Comment on attachment 8397170 [details] [diff] [review]
Fix up composition of tiled buffers

Removing the flags because more debugging shows that the regions this ends up calculating aren't right yet and I want to get this right :p
Attachment #8397170 - Flags: review?(bugmail.mozilla)
This appears to be correct in my testing.

I believe the other rendering error of odd patches of colour appearing in the low precision buffer is down to something on the client side that I'm continuing to investigate.
Attachment #8397170 - Attachment is obsolete: true
Attachment #8397305 - Flags: review?(bugmail.mozilla)
Comment on attachment 8397305 [details] [diff] [review]
Fix up composition of tiled buffers v2

Moving review over to BenWa.
Attachment #8397305 - Flags: review?(bugmail.mozilla) → review?(bgirard)
Interestingly, the problem with bad-looking contents in the low precision buffer is much worse with per-tile drawing on. I don't know what this signifies just yet.
Duplicate of this bug: 976424
Brill, the reason this is worse with per-tile drawing is that the error is client-side, and it's down to the resolution of the layer buffer not being taken into account in various areas in ValidateTile, I believe.

I'll try to fix up both paths.
I tested both paths and this seems to work correctly.
Attachment #8397903 - Flags: review?(bas)
Note: I still think there's some oddness with regards to the handling of the valid regions of high and low precision buffers, but this gets us into as good a state as we've ever been. It'll be tricky to fix everything without a comprehensive way of testing and tracking regressions.

With these patches applied, there is no more obvious brokenness on fennec for me.
tracking-fennec: ? → 30+
Comment on attachment 8397305 [details] [diff] [review]
Fix up composition of tiled buffers v2

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

::: gfx/layers/composite/TiledContentHost.cpp
@@ +382,3 @@
>    }
> +
> +  // If we aren't drawing the primary layer buffer, make sure it gets masked

I don't understand this comment. What's the primary layer buffer? Perhaps this should be:
If we're drawing the low res buffer...

I think what you really mean is:
If we're drawing the low res buffer, make sure we mask out area covered by the full res buffer to reduce overdraw.

@@ +385,5 @@
> +  // out of this draw.
> +  nsIntRegion maskRegion;
> +  if (resolution != mTiledBuffer.GetResolution()) {
> +    maskRegion = mTiledBuffer.GetValidRegion();
> +    maskRegion.ScaleRoundOut(layerScale.width, layerScale.height);

Shouldn't the mask round in?

@@ +389,5 @@
> +    maskRegion.ScaleRoundOut(layerScale.width, layerScale.height);
> +  }
> +
> +  // Make sure the resolution and difference in frame resolution are accounted
> +  // for in the layer transform.

unrelated: I wonder if we properly handle low res rotated layers. If we ever rotate around a point that isn't 0,0 I think we might not handle that transformation correctly.
Attachment #8397305 - Flags: review?(bgirard) → review+
Attachment #8397903 - Flags: review?(bas) → review+
(In reply to Benoit Girard (:BenWa) from comment #18)
> Comment on attachment 8397305 [details] [diff] [review]
> Fix up composition of tiled buffers v2
> 
> Review of attachment 8397305 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/composite/TiledContentHost.cpp
> @@ +382,3 @@
> >    }
> > +
> > +  // If we aren't drawing the primary layer buffer, make sure it gets masked
> 
> I don't understand this comment. What's the primary layer buffer? Perhaps
> this should be:
> If we're drawing the low res buffer...
> 
> I think what you really mean is:
> If we're drawing the low res buffer, make sure we mask out area covered by
> the full res buffer to reduce overdraw.

Yup, I'll make this comment clearer.

> @@ +385,5 @@
> > +  // out of this draw.
> > +  nsIntRegion maskRegion;
> > +  if (resolution != mTiledBuffer.GetResolution()) {
> > +    maskRegion = mTiledBuffer.GetValidRegion();
> > +    maskRegion.ScaleRoundOut(layerScale.width, layerScale.height);
> 
> Shouldn't the mask round in?

Yes, but region has no RoundIn :( I'll add a comment so this doesn't get (even more) confusing over time...

> @@ +389,5 @@
> > +    maskRegion.ScaleRoundOut(layerScale.width, layerScale.height);
> > +  }
> > +
> > +  // Make sure the resolution and difference in frame resolution are accounted
> > +  // for in the layer transform.
> 
> unrelated: I wonder if we properly handle low res rotated layers. If we ever
> rotate around a point that isn't 0,0 I think we might not handle that
> transformation correctly.

I don't even want to think about transforms here right now :p I've got a feeling we disable progressive rendering/low precision when there are transforms, but maybe not...
Duplicate of this bug: 984264
https://hg.mozilla.org/mozilla-central/rev/3f0dd8c73cf6
https://hg.mozilla.org/mozilla-central/rev/092a737e9451
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
This needs uplift into Beta.
Repeating comment 23 with a ni? this time - please nom for uplift
Flags: needinfo?(chrislord.net)
ack, sorry, somehow managed to miss this needinfo... I'm not sure if it's sensible to uplift this without also uplifting the work kats did after this. I'll n? him to see if that's correct or not and nominate this in the meantime.
Flags: needinfo?(chrislord.net)
Comment on attachment 8397305 [details] [diff] [review]
Fix up composition of tiled buffers v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: White flickering when panning/zooming and during animations
Testing completed (on m-c, etc.): Been on m-c for a while now
Risk to taking this patch (and alternatives if risky): Risk of replacing current brokenness with a new, different brokenness.
String or IDL/UUID changes made by this patch: None
Attachment #8397305 - Flags: approval-mozilla-beta?
Comment on attachment 8397903 [details] [diff] [review]
Fix handling of TiledLayerBuffer resolution in TiledContentClient

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: White flickering when panning/zooming and during animations
Testing completed (on m-c, etc.): Been on m-c for a while now
Risk to taking this patch (and alternatives if risky): Risk of replacing current brokenness with a new, different brokenness.
String or IDL/UUID changes made by this patch: None
Attachment #8397903 - Flags: approval-mozilla-beta?
kats, are there any other bugs related to this that will need uplifting?
Flags: needinfo?(bugmail.mozilla)
The two things I fixed (bug 993554 and bug 992218) are both already on beta. I'm not aware of anything else that needs uplifting.
Flags: needinfo?(bugmail.mozilla)
Attachment #8397305 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8397903 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.