6.57 KB, application/octet-stream
1.01 KB, patch
|Details | Diff | Splinter Review|
Created attachment 570447 [details] Demonstrate the problem with texSubImage2D on Firefox Nightly User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.2 (KHTML, like Gecko) Chrome/15.0.874.102 Safari/535.2 Steps to reproduce: I used texSubImage2D to upload image data from an Image object that has transparency. This is with Nightly running on Linux. Actual results: When using texImage2D everything works as expected, but when using texSubImage2D some of the low-alpha pixels turn dark. Expected results: The texImage2D and texSubImage2D cases should both look the same. I wrote a test program to reproduce the problem. To see the issue, unzip the files into a directory and try opening the "iconAlpha.html" in the browser. Should look fine, since this version uses texImage2D. Now open "iconAlpha.html?sub". The fringe pixels will get dark because this version uses texSubImage2D. This behavior is not seen in WebKit based browsers.
It can't be the same, since it's different for texImage* and texSubImage*. However, this should be a prerequisite for the final patch for that bug to pass.
Ah, here's this bug. Confirmed, also should be easy.
Created attachment 581936 [details] [diff] [review] Make WebGL texSubImage2D respect alpha-premultiplied DOM sources
Any chance this will get backported to FF10? It makes all the MapsGL icons look bad.
(In reply to Bill Baxter from comment #7) > Any chance this will get backported to FF10? It makes all the MapsGL icons > look bad. Extremely unlikely, in my understanding. Basically 'Release' only gets chem-spill fixes. I believe we're roughly three weeks away from 11 hitting Release, though.
I could be missing something, but there is something wrong with the code as it currently stands with this patch. This patch makes TexImage2D_dom call TexImage2D_base with srcPremultiplied = mPixelStorePremultiplyAlpha. I don't understand: mPixelStorePremultiplyAlpha, in my understanding, means "whether we should pass premultiplied values to glTexImage2D", not "whether image data passed to webgl.texImage2D is assumed to be premultiplied". Indeed, WebGL spec: http://www.khronos.org/registry/webgl/specs/latest/#PIXEL_STORAGE_PARAMETERS > UNPACK_PREMULTIPLY_ALPHA_WEBGL of type boolean > If set, then during any subsequent calls to texImage2D or texSubImage2D, the alpha channel of the source data, if present, is multiplied into the color channels during the data transfer. Notice that TexImage2D_base then calls ConvertImage with dstPremultiplied=mPixelStorePremultiplyAlpha. So the current code NEVER premultiplies or unpremultiplies DOM elements, because it always calls ConvertImage with dstPremultiplied==srcPremultiplied==mPixelStorePremultiplyAlpha.
Though, the fact that only texSubImage2D changed here and texImage2D was already like that, suggests that it's probably me not understanding this stuff correctly...
In the _dom calls, we fetch the data in the correct (un)premultiplied state. By passing in mPixelStorePremultiplyAlpha, are really telling the function 'don't mess with this, it's already what we want'. I'll file a bug to make this behavior more obvious, probably by instead of having a bool param for 'is this premultiplied', have a bool param for 'does this maybe need premuliplication'. For _dom calls, this would be false, else, true.