647802 - WebGL textures broken when decode-on-draw is enabled

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Alvaro]

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)]

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 3

[Brad Lassey [:blassey] (use needinfo?)]

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)]

Another page to look at :
http://people.mozilla.org/~bjacob/webgltexture/

Comment 5

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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)]

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 (dead)]

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]]

(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?)]

this regression was caused by bug 622470, I reverted that and webgl textures work again.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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)]

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)]

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)]

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)]

Attachment: 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)]

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)]

Comment on attachment 534830 [details] [diff] [review]
patch

Do like! But check in the version with whitespace changes ;)

Comment 17

[Wesley Johnston (:wesj)]

http://hg.mozilla.org/mozilla-central/rev/755833cb42b0

Comment 18

[Oleg Romashin (:romaxa)]

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)]

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]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/5f3571729b73

Comment 21

[Aaron Train [:aaronmt]]

(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