913999 - Canvas globalAlpha does not apply to drawWindow

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Peter J. Sloetjes]

User Agent: Mozilla/5.0 (masking-agent; rv:17.0) Gecko/17.0 Firefox/17.0 (Nightly/Aurora)
Build ID: 20130908004001

Steps to reproduce:

I used the canvas drawWindow method after setting the canvas globalAlpha to a value lower than one. (The canvas was contained in a XUL document and the window drawn was a HTML document).


Actual results:

The drawWindow output was opaque.


Expected results:

The drawWindow output should have been drawn using the specified opacity level, according to the following source: http://doxygen.db48x.net/mozilla/html/interfacensIDOMCanvasRenderingContext2D.html: "Of course, the rendering obeys the current scale, transform and globalAlpha values."

Comment 1

[Loic]

Could you attach a minimal testcase to the bug, please.

Comment 2

[Peter J. Sloetjes]

The simplest approach for testing is to modify an existing add-on, such as "Tab Scope". Several of my own restart-less add-ons use drawWindow as well (Monitor Master, Perfect View and Column Reader).

In the code, insert e.g. "ctx.globalAlpha = .2;" prior to "ctx.drawWindow(", then re-install. 

In Firefox 19 up to 25 on Windows 7, setting globalAlpha never has any effect on the output of drawWindow.

Comment 3

[Loic]

Could you attach a minimal extension showing this extension, no need to write someting complex, just a reduced testcase. Or a HTML page.

Comment 4

[Peter J. Sloetjes]

Don't be silly, drawWindow cannot be used in HTML. It really is easiest to insert the mentioned line of code into the mentioned extension. Then improve the relevant C++ code and hover the current tab for testing. In case you would rather like me to do it, I am available, and I would love to solve all those bugs with the -moz-element implementation as well (serious offer).

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to pjs.nl from comment #4)
> ...
> would rather like me to do it, I am available, and I would love to solve all
> those bugs with the -moz-element implementation as well (serious offer).

Absolutely - that'd be great.  If you get something going, just attach the patch to this bug, needinfo me and I will make sure it gets reviewed.

Comment 6

[Peter J. Sloetjes]

(In reply to Milan Sreckovic [:milan] from comment #5)
> Absolutely - that'd be great.  If you get something going, just attach the
> patch to this bug, needinfo me and I will make sure it gets reviewed.

I was thinking of a few months of paid work in someone else's office - but seriously, the mentioned features (drawWindow and -moz-element) are important, especially to extension developers, because they enable truly distinctive functionality. Please fix those outstanding bugs. I myself probably shouldn't do any more C++ coding in my own time :|

Comment 7

[Loic]

Could you attach a simple demo extension showing the issue? It would help to test and debug, especially when hundred bugs are created every day...

Comment 8

[Peter J. Sloetjes]

Attachment: TestDrawWindow.xpi provides a tab miniature popup using drawWindow

This attachment contains a bootstrapped extension that simplifies testing whether canvas drawWindow respects globalAlpha. After installation, hover a tab to see whether the window content is drawn translucently (right) or opaque (wrong) on top of the blue background.

Comment 9

[Loic]

I tested with FF26 (and FF23), the result is: http://i.imgur.com/oLsba7X.png
It looks fine, no?

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

No, that image shows the bug -- it's wrong.  The blue background, should be bleeding through the entire thing, not just showing through the border.

Comment 11

[Peter J. Sloetjes]

(In reply to Loic from comment #9)
> I tested with FF26 (and FF23), the result is: http://i.imgur.com/oLsba7X.png
> It looks fine, no?

Looks fine but is wrong - you CONFIRMED the bug ;)

Comment 12

[Peter J. Sloetjes]

Could someone please update the status of this bug?

Comment 13

[Walter Litwinczyk [:walter]]

Attachment: global_alpha

Comment 14

[Walter Litwinczyk [:walter]]

Attachment: Screenshot of the correct blending

Shows the correct blending

Comment 15

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8448989 [details] [diff] [review]
global_alpha

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

This is not my code to review.

Comment 16

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8448989 [details] [diff] [review]
global_alpha

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

::: content/canvas/src/CanvasRenderingContext2D.cpp
@@ +3658,5 @@
>    RefPtr<DrawTarget> drawDT;
> +  // Rendering directly is faster and can be done if mTarget supports Azure
> +  // and does not need alpha blending.
> +  if (gfxPlatform::GetPlatform()->SupportsAzureContentForDrawTarget(mTarget) &&
> +      GlobalAlpha() == 1.0f && NS_GET_A(backgroundColor) == 255)

Why do we need to check the background color?

Comment 17

[Walter Litwinczyk [:walter]]

I suppose it's not needed since only RenderDocument() uses backgroundColor.

Comment 18

[Walter Litwinczyk [:walter]]

Attachment: Removed background alpha check

Comment 19

[Walter Litwinczyk [:walter]]

Try: https://tbpl.mozilla.org/?tree=Try&rev=f30ff1199971

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ccee26146182

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ccee26146182

Comment 22

[Peter J. Sloetjes]

Thanks for fixing this! 
This makes it much easier to do layer composition with drawWindow results :)

And hopefully drawWindow will remain fully functional in multi-process Firefox ...