752799 - Page background goes black after panning

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Mark Funk]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1
Build ID: 20120507171005

Steps to reproduce:

1. Loaded http://www.reddit.com/r/Android/comments/ta97g/what_dont_you_like_about_android/ via a hyperlink on reddit.com/r/Android
2. Scrolled about half way down the page


Actual results:

The page's background was replaced with a solid black color making everything unreadable.


Expected results:

The page's background stays the same

Comment 1

[Mark Funk]

This happens on my Samsung Galaxy S2 Skyrocket which has an Adreno 220 GPU

Comment 2

[Catalin Suciu [:csuciu]]

I was able to reproduce this issue using the latest Nightly (2012-05-08) on 
Samsung Galaxy SII (2.3.4) and HTC Desire (2.2)

Comment 3

[Aaron Train [:aaronmt]]

Reproduced, and quite unpleasant.

--
Tested via: Nightly (05/08)
* Galaxy Nexus (Android 4.0.4), Sony Ericsson Xperia Play (Android 2.3.4)

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Unfortunately this has been a long-standing issue for a while now :(

Comment 5

[Aaron Train [:aaronmt]]

I guess something made it easier to reproduce now; I haven't seen this before at all.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

It should only happen on really long pages with a background, I think. If you feel like there is a regression here that makes this more apparent or obvious then we should bisect to find out when it happened.

Comment 7

[Catalin Suciu [:csuciu]]

This issue doesn't occur on:
Maple 13.0a1 (2012-03-12)
http://hg.mozilla.org/projects/maple/rev/e88141f7f64c

but it occurs on:
Maple 13.0a1 (2012-03-13)
http://hg.mozilla.org/projects/maple/rev/16e04eaadc14

Possible range:
http://hg.mozilla.org/projects/maple/pushloghtml?fromchange=e88141f7f64c&tochange=16e04eaadc14

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Let's see if this is something we can make better (not use a black background), even if we can't fix the real bug.

Comment 9

[Joe Drew (not getting mail)]

Well, it doesn't make much sense, but...

The first bad revision is:
changeset:   89088:a5980ce21d40
user:        Kartikaya Gupta <[e-mail]>
date:        Mon Mar 12 12:03:38 2012 -0400
summary:     Bug 732564 - Distinguish between changes where we should sync-update the Java viewport and where the compositor has to update Java. r=Cwiiis

Comment 10

[Joe Drew (not getting mail)]

qawanted for two reasons: one is for a reduced testcase; alternately, a different example of this on the internet, as I can't reproduce it reliably on Reddit any more :(

Comment 11

[Joe Drew (not getting mail)]

I can, however, reliably reproduce this on Aurora, strangely enough.

Comment 12

[Joe Drew (not getting mail)]

Ah, but I can reproduce this on Nightly on http://nomnompaleo.com

Comment 13

[Tony Chung [:tchung]]

testcase wanted, but we need to reliable reproduce this first.

Comment 14

[Martijn Wargers (dead)]

Attachment: testcase

Tap on the link (or just scroll down a while) to get the black background.
The background that is used in the testcase is pointing to http://static.tumblr.com/fftf9xi/27ll9xup0/texture_main.jpg which is a rather large image.

Comment 15

[Joe Drew (not getting mail)]

pixman's analyze_extent is returning FALSE here, which means that we're just trying to draw beyond the range pixman supports. This will need a fix in layout. The coordinates need to not be too big.

I can reproduce very similar problems on GDI (i.e., bug 671302) due to the same underlying limitation in pixman.

Comment 16

[Jet Villegas (inactive)]

I'm not sure how we'd fix this in Layout as there isn't a single bottleneck to check for extents prior to a paint. I think our best bet here is to implement comment 8 and paint the default HTML background color and not black when we overflow the extents.

Comment 17

[Jeff Muizelaar [:jrmuizel]]

I'll try something.

Comment 18

[Jeff Muizelaar [:jrmuizel]]

Attachment: This seems to do the trick

Comment 19

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 627348 [details] [diff] [review]
This seems to do the trick

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

::: gfx/thebes/gfxUtils.cpp
@@ +513,5 @@
>  
> +    gfxMatrix m = aUserSpaceToImageSpace;
> +    if (doTile) {
> +        m.y0 = fmod(m.y0, aImageRect.height);
> +        m.x0 = fmod(m.x0, aImageRect.height);

Should this one be aImageRect.width?

Comment 20

[Jeff Muizelaar [:jrmuizel]]

Attachment: Better

Comment 21

[Jeff Muizelaar [:jrmuizel]]

Attachment: Move the translation within pixman's range

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 627605 [details] [diff] [review]
Move the translation within pixman's range

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

::: gfx/thebes/gfxUtils.cpp
@@ +518,1 @@
>  #endif

Use #ifdef MOZ_OPTIMIZE_MOBILE ... #else ... #endif instead of #ifndef

Comment 23

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b82ef4adc4d8

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b82ef4adc4d8

Comment 25

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 627605 [details] [diff] [review]
Move the translation within pixman's range

[Approval Request Comment]
Bug caused by (feature/regressing bug #): pixman coordinate limits
User impact if declined: black backgrounds instead of images on some pages
Testing completed (on m-c, etc.): been on m-c for quite a while
Risk to taking this patch (and alternatives if risky): mobile only, perhaps some image drawing wrongness.
String or UUID changes made by this patch: None

Comment 26

[Joe Drew (not getting mail)]

Comment on attachment 627605 [details] [diff] [review]
Move the translation within pixman's range

This is already on Aurora. Please land on beta ASAP.

Comment 27

[Joe Drew (not getting mail)]

http://hg.mozilla.org/releases/mozilla-beta/rev/23daca1b1aa7