Closed
Bug 952354
Opened 11 years ago
Closed 11 years ago
Draw raster image frames even if they were decoded with nonpremultiplied alpha if the frame is opaque
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: seth, Assigned: seth)
References
Details
Attachments
(2 files)
1.53 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
3.68 KB,
patch
|
seth
:
review+
|
Details | Diff | Splinter Review |
Right now we can hit some horrible cases where the same image is used by both WebGL and an <img> tag or similar. WebGL often requests that we decode the image with non-premultiplied alpha, while when displaying content images in HTML and CSS we decode with premultiplied alpha. This can result in us discarding and redecoding the image twice *every time we redraw the page*, since we can only store one decoded version at a time and each time a different version is requested we throw the other one away.
The real fix here is to make it possible to store multiple decoded versions of the same image. (We need that for so many reasons!) However, we can handle many instances of mixed WebGL/HTML content inadvertently hitting this path by being willing to draw image frames with non-premultiplied alpha if the frame is opaque anyway. In this case premultiplied/non-premultiplied should make no difference.
We hit this issue in the real world with Goo Create, so it's not a theoretical concern.
Assignee | ||
Comment 1•11 years ago
|
||
This patch makes us just go ahead and draw in the cases where we have a non-premultipled alpha but opaque frame.
This gets rid of the endless redecoding visible on Goo Create.
Attachment #8350420 -
Flags: review?(jmuizelaar)
Comment 2•11 years ago
|
||
Comment on attachment 8350420 [details] [diff] [review]
Draw non-premultiplied alpha raster image frames if the frame is opaque.
Ok. We really should make sure we fix this properly though. Someone else will undoubtedly hit the same cliff.
Attachment #8350420 -
Flags: review?(jmuizelaar) → review+
Comment 3•11 years ago
|
||
If the decode with default flags is the first one to get started then we should do the same in GetFrame/CopyFrame so we don't have to do an extra decode.
Attachment #8358848 -
Flags: review?(seth)
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8358848 [details] [diff] [review]
Same idea for GetFrame and CopyFrame
Review of attachment 8358848 [details] [diff] [review]:
-----------------------------------------------------------------
Even better. Thanks, Timothy.
Attachment #8358848 -
Flags: review?(seth) → review+
Assignee | ||
Comment 5•11 years ago
|
||
I'd like to land these together. Timothy, have you run a try job on yours? (I have run one on my patch, which is posted in bug 943803.)
Flags: needinfo?(tnikkel)
Comment 6•11 years ago
|
||
I have run try job, it was good.
I was actually going to suggest holding off on landing these because it seems like we have some bugs when webgl tries to use images as textures and the resultant decode/discard stuff because of premultiplied alpha. There is also bug 958758. Landing the patches in this bug would make those happen much more rarely, so we could have an initial period where we shake out any bugs and then land these.
Flags: needinfo?(tnikkel)
Assignee | ||
Comment 8•11 years ago
|
||
How's this for an answer?
https://hg.mozilla.org/integration/mozilla-inbound/rev/bac8b8ba264e
Flags: needinfo?(seth)
Comment 9•11 years ago
|
||
And pushed my patch too
https://hg.mozilla.org/integration/mozilla-inbound/rev/11f1efb21790
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bac8b8ba264e
https://hg.mozilla.org/mozilla-central/rev/11f1efb21790
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•