Closed Bug 698169 Opened 8 years ago Closed 8 years ago

[WebGL] texSubImage2D with an Image seems to copy premultiplied alpha values

Categories

(Firefox :: General, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 11

People

(Reporter: wbaxter, Assigned: jgilbert)

References

Details

Attachments

(2 files)

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.
Jeff, is this the same as Bug 696569 ?
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.
Depends on: 696569
Ah, here's this bug. Confirmed, also should be easy.
Assignee: nobody → jgilbert
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #581936 - Flags: review?(bjacob) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/43aeb296b99e
Blocks: 696569
No longer depends on: 696569
Target Milestone: --- → Firefox 11
https://hg.mozilla.org/mozilla-central/rev/43aeb296b99e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Blocks: 738757
You need to log in before you can comment on or make changes to this bug.