Last Comment Bug 647802 - WebGL textures broken when decode-on-draw is enabled
: WebGL textures broken when decode-on-draw is enabled
Status: VERIFIED FIXED
: 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
:
:
Mentors:
: 638272 (view as bug list)
Depends on: 740841 622470
Blocks:
  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: ---


Attachments
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 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: http://support.mozilla.com/es/questions/799056



Reproducible: Always

Steps to Reproduce:
Any WebGL using textures
Actual Results:  
Black objects.
Comment 1 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 http://www.khronos.org/webgl/wiki/WebGL_and_OpenGL
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 :: https://cvs.khronos.org/svn/repos
/registry/trunk/public/webgl/sdk/demos/google/image-texture-test/demo.js ::
<TOP_LEVEL> :: line 213" data:no]

Note:
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 Wesley Johnston (:wesj) 2011-04-06 10:52:53 PDT
*** Bug 638272 has been marked as a duplicate of this bug. ***
Comment 3 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 Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-04-07 09:34:30 PDT
Another page to look at :
http://people.mozilla.org/~bjacob/webgltexture/
Comment 5 Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-04-07 10:13:10 PDT
Odd.  http://people.mozilla.org/~bjacob/webgltexture/ WFM in the 04/05 build, I noted it because of the original message in the forums had mentioned it.

http://www.khronos.org/webgl/wiki/WebGL_and_OpenGL 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 khronos.org page.
Comment 6 Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-04-07 10:52:07 PDT
http://www.khronos.org/webgl/wiki/WebGL_and_OpenGL works on 2/18 build, doesn't work on 2/19 build.
Comment 7 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-04-07 12:16:30 PDT
Bonsai change log:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2011-02-18+04%3A00%3A00&enddate=2011-02-19+04%3A00%3A00
Regression from bug 635275?
Comment 8 Jeff Muizelaar [:jrmuizel] 2011-04-08 09:57:08 PDT
(In reply to comment #7)
> Bonsai change log:
> http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2011-02-18+04%3A00%3A00&enddate=2011-02-19+04%3A00%3A00
> 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 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 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 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))
      return NS_ERROR_NOT_AVAILABLE;
    if (!CanForciblyDiscard() || mDecoder || mAnim)
      return NS_ERROR_NOT_AVAILABLE;

since !CanForciblyDiscard(), which is false since !mDecoded.
Comment 12 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 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 Alon Zakai (:azakai) 2011-05-24 10:40:46 PDT
Created attachment 534830 [details] [diff] [review]
patch

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 Oleg Romashin (:romaxa) 2011-05-25 17:32:33 PDT
Comment on attachment 534830 [details] [diff] [review]
patch

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 :: http://learningwebgl.com/lessons/lesson05/index.html :: handleLoadedTexture :: line 126"  data: no]
Comment 16 Joe Drew (not getting mail) 2011-05-30 11:55:09 PDT
Comment on attachment 534830 [details] [diff] [review]
patch

Do like! But check in the version with whitespace changes ;)
Comment 17 Wesley Johnston (:wesj) 2011-05-31 13:39:04 PDT
http://hg.mozilla.org/mozilla-central/rev/755833cb42b0
Comment 18 Oleg Romashin (:romaxa) 2011-06-10 07:37:25 PDT
Comment on attachment 534830 [details] [diff] [review]
patch

Asking flag due: https://bugzilla.mozilla.org/show_bug.cgi?id=606218#c20
Comment 19 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 Daniel Holbert [:dholbert] 2011-06-23 15:05:10 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/5f3571729b73
Comment 21 Aaron Train [:aaronmt] 2011-06-30 10:52:37 PDT
(In reply to comment #17)
> http://hg.mozilla.org/mozilla-central/rev/755833cb42b0

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

(In reply to comment #20)
> http://hg.mozilla.org/releases/mozilla-aurora/rev/5f3571729b73

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.