Last Comment Bug 752799 - Page background goes black after panning
: Page background goes black after panning
Status: RESOLVED FIXED
[layout]
: regression, reproducible, testcase
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: mozilla15
Assigned To: Jeff Muizelaar [:jrmuizel]
:
:
Mentors:
http://www.reddit.com/r/Android/comme...
Depends on:
Blocks: 732564
  Show dependency treegraph
 
Reported: 2012-05-07 21:45 PDT by Mark Funk
Modified: 2014-04-25 10:31 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
+


Attachments
testcase (346 bytes, text/html)
2012-05-21 16:21 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
This seems to do the trick (631 bytes, patch)
2012-05-25 13:56 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
Better (630 bytes, patch)
2012-05-25 14:12 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
Move the translation within pixman's range (1.93 KB, patch)
2012-05-27 18:25 PDT, Jeff Muizelaar [:jrmuizel]
roc: review+
joe: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Mark Funk 2012-05-07 21:45:40 PDT
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 2012-05-07 21:49:44 PDT
This happens on my Samsung Galaxy S2 Skyrocket which has an Adreno 220 GPU
Comment 2 Catalin Suciu [:csuciu] 2012-05-08 05:18:06 PDT
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] 2012-05-08 06:46:30 PDT
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:kats@mozilla.com) 2012-05-08 07:20:53 PDT
Unfortunately this has been a long-standing issue for a while now :(

*** This bug has been marked as a duplicate of bug 671302 ***
Comment 5 Aaron Train [:aaronmt] 2012-05-08 07:26:14 PDT
I guess something made it easier to reproduce now; I haven't seen this before at all.
Comment 6 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-08 07:42:53 PDT
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] 2012-05-09 07:19:14 PDT
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?) 2012-05-10 12:29:44 PDT
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) 2012-05-15 14:51:30 PDT
Well, it doesn't make much sense, but...

The first bad revision is:
changeset:   89088:a5980ce21d40
user:        Kartikaya Gupta <kgupta@mozilla.com>
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) 2012-05-18 10:58:52 PDT
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) 2012-05-18 11:12:47 PDT
I can, however, reliably reproduce this on Aurora, strangely enough.
Comment 12 Joe Drew (not getting mail) 2012-05-18 11:15:08 PDT
Ah, but I can reproduce this on Nightly on http://nomnompaleo.com
Comment 13 Tony Chung [:tchung] 2012-05-21 09:30:01 PDT
testcase wanted, but we need to reliable reproduce this first.
Comment 14 Martijn Wargers [:mwargers] (not working for Mozilla) 2012-05-21 16:21:13 PDT
Created attachment 625814 [details]
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) 2012-05-24 13:45:21 PDT
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 (:jet) 2012-05-24 17:34:15 PDT
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] 2012-05-25 10:16:16 PDT
I'll try something.
Comment 18 Jeff Muizelaar [:jrmuizel] 2012-05-25 13:56:02 PDT
Created attachment 627348 [details] [diff] [review]
This seems to do the trick
Comment 19 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-25 14:06:26 PDT
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] 2012-05-25 14:12:44 PDT
Created attachment 627355 [details] [diff] [review]
Better
Comment 21 Jeff Muizelaar [:jrmuizel] 2012-05-27 18:25:24 PDT
Created attachment 627605 [details] [diff] [review]
Move the translation within pixman's range
Comment 22 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-05-27 19:24:38 PDT
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] 2012-05-28 08:49:13 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/b82ef4adc4d8
Comment 24 Ed Morley [:emorley] 2012-05-29 10:21:45 PDT
https://hg.mozilla.org/mozilla-central/rev/b82ef4adc4d8
Comment 25 Jeff Muizelaar [:jrmuizel] 2012-06-04 14:30:35 PDT
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) 2012-06-05 11:53:33 PDT
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) 2012-06-05 13:48:26 PDT
http://hg.mozilla.org/releases/mozilla-beta/rev/23daca1b1aa7

Note You need to log in before you can comment on or make changes to this bug.