Closed Bug 672646 Opened 11 years ago Closed 11 years ago

Radial gradients in <canvas> are opaque (regression from Azure)

Categories

(Core :: Graphics, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla8
Tracking Status
firefox7 + fixed

People

(Reporter: benjamin, Assigned: jrmuizel)

References

Details

(Keywords: regression)

Attachments

(4 files)

Attached image 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
Assignee: nobody → bas.schouten
Can you reproduce something similar in a canvas in an html document?
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
Attached file Web-content testcase
Yes, I can reproduce this with plain web content as well, HTML testcase attached.
Assignee: bas.schouten → jmuizelaar
It looks like the cause of this is us assuming premultiplied data when the data is not actually premultiplied. The fix should be easy.
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.
Attachment #546944 - Flags: review?
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.
(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 on attachment 546944 [details] [diff] [review]
Premultiply before blending

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

Yeah, OK.
Attachment #546944 - Flags: review? → review+
Attached patch reftestSplinter Review
Attachment #547160 - Flags: review?(roc)
Comment on attachment 547160 [details] [diff] [review]
reftest

Review of attachment 547160 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #547160 - Flags: review?(roc) → review+
Attachment #546944 - Flags: approval-mozilla-aurora?
Attachment #547160 - Flags: approval-mozilla-aurora?
http://hg.mozilla.org/mozilla-central/rev/6d355f495e8d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Depends on: 674003
Keywords: regression
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?
Summary: Radial gradients in <canvas> on glass (Aero Window Title extension) are opaque (regression from Azure) → Radial gradients in <canvas> are opaque (regression from Azure)
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 on attachment 546944 [details] [diff] [review]
Premultiply before blending

Approved for releases/mozilla-aurora
Attachment #546944 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #547160 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.