The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Firefox 11

Status

()

Firefox
General
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Bill Baxter, Assigned: jgilbert)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 11
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
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.
Jeff, is this the same as Bug 696569 ?
Blocks: 680721
(Assignee)

Comment 2

5 years ago
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
(Assignee)

Comment 3

5 years ago
Ah, here's this bug. Confirmed, also should be easy.
Assignee: nobody → jgilbert
(Assignee)

Comment 4

5 years ago
Created attachment 581936 [details] [diff] [review]
Make WebGL texSubImage2D respect alpha-premultiplied DOM sources
Attachment #581936 - Flags: review?(bjacob)
(Assignee)

Updated

5 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #581936 - Flags: review?(bjacob) → review+
(Assignee)

Comment 5

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

5 years ago
Any chance this will get backported to FF10?  It makes all the MapsGL icons look bad.
(Assignee)

Comment 8

5 years ago
(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...
(Assignee)

Comment 11

5 years ago
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
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Blocks: 738757
You need to log in before you can comment on or make changes to this bug.