Last Comment Bug 647802 - WebGL textures broken when decode-on-draw is enabled
: WebGL textures broken when decode-on-draw is enabled
: verified-aurora
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: Other Other
-- normal (vote)
: Firefox 6
Assigned To: Nobody; OK to take it and work on it
: 638272 (view as bug list)
Depends on: 740841 622470
  Show dependency treegraph
Reported: 2011-04-05 04:17 PDT by Alvaro
Modified: 2012-04-02 11:19 PDT (History)
14 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

patch (1.31 KB, patch)
2011-05-24 10:40 PDT, Alon Zakai (:azakai)
joe: review+
romaxa: feedback+
bugzilla: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Alvaro 2011-04-05 04:17:20 PDT
User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mobile Firefox 4 beta 13

In Moble Firefox WebGL using textures does not work anymore on a Samsung Galaxy Tab. It's as if the texture was black. Non textured meshes are fine.

Textures worked fine weeks or a month or so ago with Fennec (don't know the version, sorry), until we updated.

Some other people also reported this here:

Reproducible: Always

Steps to Reproduce:
Any WebGL using textures
Actual Results:  
Black objects.
Comment 1 User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-04-05 14:23:45 PDT
Steps to repro:

Mozilla/5.0 (Android; Linux armv71; rv2.2a1pre) Gecko/20110405 Firefox/4.2a1pre Fennec/4.1a1pre
Device: Droid 2 
OS: Android 2.2

1. go to
2. open the link called : image-texture-test example in a new tab

Expected: the new tab will show like the sample image in the first page
Actual: only the text : "Texture test" appears, but not the blue boxes and the white wording; Console shows error : 
Error: uncaught exception: [Exception... "Failure arg 5 [nsIDOMWebGLRenderingContext.texImage2D]" nsresult: "0x80004005
(NS_ERROR_FAILURE)" location: "JS frame ::
/registry/trunk/public/webgl/sdk/demos/google/image-texture-test/demo.js ::
<TOP_LEVEL> :: line 213" data:no]

1. works fine (no error in console) on desktop version of Firefox :
Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.2a1pre) Gecko/20110331 Firefox/4.2a1pre
Comment 2 User image Wesley Johnston (:wesj) 2011-04-06 10:52:53 PDT
*** Bug 638272 has been marked as a duplicate of this bug. ***
Comment 3 User image Brad Lassey [:blassey] (use needinfo?) 2011-04-06 10:56:53 PDT
some useful information from vlad in bug 638272:

> Works fine in 4.0b5, in 4.0b6pre (20110302) a bunch of the textures don't load.
>  The teximage2D call is returning a NS_ERROR_FAILURE, bad arg 5.  I'm guessing
> we're failing somehow to convert the DOM image correctly, though again it works
> fine in 4.0b5.
Comment 4 User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-04-07 09:34:30 PDT
Another page to look at :
Comment 5 User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-04-07 10:13:10 PDT
Odd. WFM in the 04/05 build, I noted it because of the original message in the forums had mentioned it. is still broken for me with the release:
Mozilla/5.0 (Android; Linux armv71; rv:2.1) Gecko20110318 Firefox/4.0b13pre Fennec/4.0
Device: Droid 2
OS: Android 2.2

Still looking for regression range for the page.
Comment 6 User image Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-04-07 10:52:07 PDT works on 2/18 build, doesn't work on 2/19 build.
Comment 7 User image Martijn Wargers [:mwargers] 2011-04-07 12:16:30 PDT
Bonsai change log:
Regression from bug 635275?
Comment 8 User image Jeff Muizelaar [:jrmuizel] 2011-04-08 09:57:08 PDT
(In reply to comment #7)
> Bonsai change log:
> Regression from bug 635275?

This regression range doesn't have anything specific. Can we get a regression range with changeset ids instead of dates?
Comment 9 User image Brad Lassey [:blassey] (use needinfo?) 2011-04-08 13:57:25 PDT
this regression was caused by bug 622470, I reverted that and webgl textures work again.
Comment 10 User image Vladimir Vukicevic [:vlad] [:vladv] 2011-04-08 21:09:45 PDT
blassey: (from irc) I would have tested this on either a Galaxy S, Galaxy Tab, or a Nexus One -- but most likely one of the first two.

WebGL uses a slightly different path through imglib than the rest of the code -- see bug 622184 .  Nothing in there jumped out at me as having been affected by decode on draw (I was thinking of it when I wrote it), but that doesn't necessarily mean much.  Stepping through webgl's TexImage2D on one of the simple testcases should pretty quickly show what's failing along the way.
Comment 11 User image Alon Zakai (:azakai) 2011-04-28 10:13:36 PDT
The failure happens in RasterImage::GetFrame.

We enter this block

  if (desiredDecodeFlags != mFrameDecodeFlags) {

since FLAG_SYNC_DECODE is in aFlags but not desiredDecodeFlags (6 vs 7). And then in that block, we hit the last line here

    // if we can't discard, then we're screwed; we have no way
    // to re-decode.  Similarly if we aren't allowed to do a sync
    // decode.
    if (!(aFlags & FLAG_SYNC_DECODE))
    if (!CanForciblyDiscard() || mDecoder || mAnim)

since !CanForciblyDiscard(), which is false since !mDecoded.
Comment 12 User image Alon Zakai (:azakai) 2011-05-18 11:50:03 PDT
I'm not familiar enough with the decode on draw stuff to know what is going wrong in comment 11. Oleg?
Comment 13 User image Oleg Romashin (:romaxa) 2011-05-18 19:35:22 PDT
with decode on draw we should decode image as soon as it required, upload to GPU and forget decoded surface (keep only jpeg/gif compressed data...), if it is does not work this way, then some other logic broken in surrounding code.
Comment 14 User image Alon Zakai (:azakai) 2011-05-24 10:40:46 PDT
Created attachment 534830 [details] [diff] [review]

Patch without whitespace. It just adds a check for mDecoded and some indentation.

The problem was that GetFrame assumed there was already something decoded. It then tries to forcibly discard that if it isn't a perfect match to what we are looking for. With decode-on-draw, there is nothing decoded, and discarding fails, which is what we were hitting. But there is no need to discard anything if nothing was decoded, which is what the patch does.

Looks good on try.
Comment 15 User image Oleg Romashin (:romaxa) 2011-05-25 17:32:33 PDT
Comment on attachment 534830 [details] [diff] [review]

Works fine for me with this patch, without it just giving 
JavaScript error: , line 0: uncaught exception: [Exception... "Failure arg 5 [nsIDOMWebGLRenderingContext.texImage2D]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: :: handleLoadedTexture :: line 126"  data: no]
Comment 16 User image Joe Drew (not getting mail) 2011-05-30 11:55:09 PDT
Comment on attachment 534830 [details] [diff] [review]

Do like! But check in the version with whitespace changes ;)
Comment 17 User image Wesley Johnston (:wesj) 2011-05-31 13:39:04 PDT
Comment 18 User image Oleg Romashin (:romaxa) 2011-06-10 07:37:25 PDT
Comment on attachment 534830 [details] [diff] [review]

Asking flag due:
Comment 19 User image Alon Zakai (:azakai) 2011-06-10 09:50:36 PDT
I second the request for Aurora approval. This is a very small patch, is very low risk, and it unbreaks WebGL on mobile. I also suspect it fixes the same problem on desktop, but just for users that modified the decode on draw flag themselves.
Comment 20 User image Daniel Holbert [:dholbert] (vacation, returning 2/27) 2011-06-23 15:05:10 PDT
Comment 21 User image Aaron Train [:aaronmt] 2011-06-30 10:52:37 PDT
(In reply to comment #17)

Verified Fixed
Mozilla/5.0 (Android; Linux armv7l; rv:7.0a1) Gecko/20110630 Firefox/7.0a1 Fennec/7.0a1

(In reply to comment #20)

Verified Fixed
Mozilla/5.0 (Android; Linux armv7l; rv:6.0a2) Gecko/20110630 Firefox/6.0a2 Fennec/6.0a2

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