Last Comment Bug 672646 - Radial gradients in <canvas> are opaque (regression from Azure)
: Radial gradients in <canvas> are opaque (regression from Azure)
Status: VERIFIED FIXED
: regression
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86 Windows 7
: -- normal (vote)
: mozilla8
Assigned To: Jeff Muizelaar [:jrmuizel]
:
Mentors:
Depends on: 674003
Blocks: 666097
  Show dependency treegraph
 
Reported: 2011-07-19 14:19 PDT by Benjamin Smedberg [:bsmedberg]
Modified: 2013-12-27 14:20 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
Screenshot (70.25 KB, image/png)
2011-07-19 14:19 PDT, Benjamin Smedberg [:bsmedberg]
no flags Details
Web-content testcase (466 bytes, text/html)
2011-07-19 14:29 PDT, Benjamin Smedberg [:bsmedberg]
no flags Details
Premultiply before blending (208.46 KB, patch)
2011-07-19 17:16 PDT, Jeff Muizelaar [:jrmuizel]
roc: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Review
reftest (2.21 KB, patch)
2011-07-20 11:21 PDT, Jeff Muizelaar [:jrmuizel]
roc: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Review

Description Benjamin Smedberg [:bsmedberg] 2011-07-19 14:19:16 PDT
Created attachment 546905 [details]
Screenshot

In the Aero Window Title extension, I use a canvas to draw the shadow around the window title. The radial gradients I draw, which are a gradient from rgba(100%, 100%, 100%, 0.3)  to rgba(100%, 100%, 100%, 0) are now drawing completely opaque. Screenshot attached. The extension in question is https://addons.mozilla.org/en-US/firefox/addon/aero-window-title/ and has a screenshot of how it should look.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110719 Firefox/8.0a1
Comment 1 Jeff Muizelaar [:jrmuizel] 2011-07-19 14:21:17 PDT
Can you reproduce something similar in a canvas in an html document?
Comment 2 Benjamin Smedberg [:bsmedberg] 2011-07-19 14:21:48 PDT
And I have verified that disabling azure via pref fixes the problem.

The code which draws these gradients can be seen at http://hg.mozilla.org/users/bsmedberg_mozilla.com/aero-window-title/file/5ef85c7389dc/aero-overlay.xul#l50
Comment 3 Benjamin Smedberg [:bsmedberg] 2011-07-19 14:29:51 PDT
Created attachment 546911 [details]
Web-content testcase

Yes, I can reproduce this with plain web content as well, HTML testcase attached.
Comment 4 Jeff Muizelaar [:jrmuizel] 2011-07-19 15:57:34 PDT
It looks like the cause of this is us assuming premultiplied data when the data is not actually premultiplied. The fix should be easy.
Comment 5 Jeff Muizelaar [:jrmuizel] 2011-07-19 17:16:58 PDT
Created attachment 546944 [details] [diff] [review]
Premultiply before blending

I don't know who would be a good reviewer for this. It's all pretty straightforward so I'll take a review from anyone who's willing to give it.
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-07-19 17:24:30 PDT
Wouldn't it make more sense to have DrawTargetD2D::CreateGradientTexture create the texture already premultiplied? Less work per pixel.

Besides, we probably should be interpolating in premultiplied space.
Comment 7 Jeff Muizelaar [:jrmuizel] 2011-07-19 17:29:29 PDT
(In reply to comment #6)
> Wouldn't it make more sense to have DrawTargetD2D::CreateGradientTexture
> create the texture already premultiplied? Less work per pixel.

Yeah, but more non parallelized work on the CPU. No-one will notice an extra multiply per pixel on the GPU. If we really cared we could even hide the multiply in the output merger and no-one would ever notice it there.

> Besides, we probably should be interpolating in premultiplied space.

Afaict, gradients aren't interpolated in premultiplied space.
Comment 8 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-07-19 19:36:46 PDT
Comment on attachment 546944 [details] [diff] [review]
Premultiply before blending

Review of attachment 546944 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, OK.
Comment 9 Jeff Muizelaar [:jrmuizel] 2011-07-20 11:21:42 PDT
Created attachment 547160 [details] [diff] [review]
reftest
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-07-20 13:25:16 PDT
Comment on attachment 547160 [details] [diff] [review]
reftest

Review of attachment 547160 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 11 :Ehsan Akhgari (busy, don't ask for review please) 2011-07-22 14:11:39 PDT
http://hg.mozilla.org/mozilla-central/rev/6d355f495e8d
Comment 12 :Ehsan Akhgari (busy, don't ask for review please) 2011-07-22 14:15:33 PDT
Also: http://hg.mozilla.org/mozilla-central/rev/a9e310db3ac0
Comment 13 Asa Dotzler [:asa] 2011-07-28 14:40:41 PDT
We discussed this in triage and we need risk/reward on this bug and we need an honest assessment of whether Azure is complete enough to ship in 7.  Is this a blocker to shipping Azure canvas (are semi-transparent radial gradients in canvas used in the wild enough that we wouldn't want to ship this bug?) How confident are we about this fix and are there any additional outstanding Azure canvas bugs that will be requesting approval in the coming days or weeks?
Comment 14 Jeff Muizelaar [:jrmuizel] 2011-07-28 14:56:23 PDT
The risk of this patch is quite low, it only impacts an area that is not as commonly used and the fix can only have an impact in that area.

There was a small bug in it (bug 674003) but the problem there is even more rare and the resulting rendering less bad.

These are the only two issues with Azure that I've seen so far.

I would say that this issue would probably block shipping Azure in 7 and given how tiny the fix is it would be a mistake to disable Azure because we didn't take this.
Comment 15 christian 2011-08-02 14:50:54 PDT
Comment on attachment 546944 [details] [diff] [review]
Premultiply before blending

Approved for releases/mozilla-aurora
Comment 16 Jeff Muizelaar [:jrmuizel] 2011-08-04 15:11:03 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/ea86edb8c31c
Comment 17 AndreiD[QA] 2011-08-25 04:58:34 PDT
Build ID: Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Setting this bug as Verified. The issue reported is fixed on Fx7 Beta. Tested following the steps in the description.

However, it's unclear what changes should be visible if loading the "Web-content testcase" html? Thanks

Note You need to log in before you can comment on or make changes to this bug.