Last Comment Bug 675210 - Some pages entirely black on Fennec when GL Layers enabled
: Some pages entirely black on Fennec when GL Layers enabled
Status: RESOLVED FIXED
[inbound]
: regression
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: mozilla8
Assigned To: Ali Juma [:ajuma]
:
Mentors:
Depends on:
Blocks: opengl-mobile
  Show dependency treegraph
 
Reported: 2011-07-29 07:57 PDT by Ali Juma [:ajuma]
Modified: 2011-08-04 03:19 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Recover from incorrect guess of surface format (1.64 KB, patch)
2011-07-29 18:10 PDT, Ali Juma [:ajuma]
no flags Details | Diff | Splinter Review
Defer allocation of textures (2.56 KB, patch)
2011-08-03 07:59 PDT, Ali Juma [:ajuma]
matt.woodrow: review+
Details | Diff | Splinter Review

Description Ali Juma [:ajuma] 2011-07-29 07:57:55 PDT
This is occurring sporadically with several sites, but is most consistently reproducible at https://addons.mozilla.org/en-US/mobile/

The page appears entirely black, with no content displayed. 

In some cases (e.g. at m.cnn.com), the content briefly flashes on the screen before disappearing, and reappears if the page is scrolled to the bottom.

This seems to be a very recent regression, and indeed the issue does not occur when the patches from bug 660090 are backed out.
Comment 1 Benoit Girard (:BenWa) 2011-07-29 08:55:05 PDT
Unless Joe has an idea what's causing this I suggest we back them out until we can fix them up.
Comment 2 Ali Juma [:ajuma] 2011-07-29 16:48:33 PDT
The problem turns out to be that in certain cases, the wrong internal format is used in fTexImage2D. This should be easy to fix -- patch coming soon.
Comment 3 Benoit Girard (:BenWa) 2011-07-29 16:54:37 PDT
Is it bug 661002 by any chance?
Comment 4 Ali Juma [:ajuma] 2011-07-29 18:03:24 PDT
(In reply to comment #3)
> Is it bug 661002 by any chance?

It certainly seems related. What's happening is that with bug 660090, the constructor of TextureImageEGL makes a call to Resize(), that in turn calls fTexImage2D. The latter call is made under the assumption that the surface's format is mUpdateFormat. When this guess turns out to be wrong, we're in trouble.

Specifically, we're running into a situation where mUpdateFormat is ARGB_32, but the surface passed to TextureImageEGL::DirectUpdate() is RGB16_565. At this point the texture has already been allocated with internal format LOCAL_GL_RGBA, but it should really be LOCAL_GL_RGB.
Comment 5 Ali Juma [:ajuma] 2011-07-29 18:10:48 PDT
Created attachment 549532 [details] [diff] [review]
Recover from incorrect guess of surface format

Before calling UploadSurfaceToTexture, we check if our guess of the surface format turned out to be incorrect; if so, we ask UploadSurfaceToTexture to reallocate the texture (using the correct format).
Comment 6 Matt Woodrow (:mattwoodrow) 2011-07-29 20:30:04 PDT
This seems like it will happen fairly often on mobile, and probably isn't great for performance.

Can we either improve our guessing logic, or defer the initial texture creation until we know the surface type?
Comment 7 Ali Juma [:ajuma] 2011-08-03 07:59:03 PDT
Created attachment 550389 [details] [diff] [review]
Defer allocation of textures

This postpones the allocation of textures until the texture is about to be used. 

This is essentially a middle-ground between what was happening before bug 660090 landed (we were allocating textures too late, and hence running into incomplete framebuffer errors), and what is happening currently (we are allocating textures early and hence sometimes we don't choose the right internal format).
Comment 8 Matt Woodrow (:mattwoodrow) 2011-08-03 13:00:48 PDT
Comment on attachment 550389 [details] [diff] [review]
Defer allocation of textures

I'd r+ this twice if I could. :)
Comment 9 Benoit Girard (:BenWa) 2011-08-03 13:31:36 PDT
Nice! Lets get this landed :)
Comment 11 Marco Bonardo [::mak] 2011-08-04 03:19:45 PDT
http://hg.mozilla.org/mozilla-central/rev/c048ca40dcd1

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