691571 - Canvas 2D hardware acceleration makes createPattern flicker

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[David Woldrich]

Attachment: flickeryrace.zip

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0.1) Gecko/20100101 Firefox/7.0.1
Build ID: 20110928134238

Steps to reproduce:

go to

https://clubcompy.com

watch the demo marquee display the demos on windows 7 on FF 7.0.1 with "Use hardware acceleration when available" enabled.


Actual results:

Some of the demos which draw characters to the canvas were failing intermittently.  If I unchecked "use hardware acceleration when available" in the advanced options, then the character scenes draw normally.


Expected results:

The scenes should draw correctly/identically whether hardware acceleration is enabled or not.  The characters at clubcompy.com are drawn using patterns acquired using context2d.createPattern(canvas, "repeat").  ClubCompy.com caches up to 2048 of these patterns.  The patterns are used in a fillRect immediately after being created, so the code needs to wait for the hardware to build the pattern before trying to paint with it (if that is what's causing the pattern paint failure.)

I'm the creator of clubcompy.com, so perhaps I can help dissect this issue.  Please feel free to contact me at my email if you would like to discuss, or I can join you on IRC to discuss.

I'm going to leave the pattern code enabled for the time being on the clubcompy site, assuming this is a problem you can solve easily.  If I don't see a fix pushed in a FF update in the next month or so, I'll disable the createPattern code by default and give you some query string parameter to enable the createPattern code lines.

If it helps, I'm running with an nVidia GeForce GTX 560 Ti, Windows 7 Ultimate, SP 1

Comment 1

[Bas Schouten (:bas.schouten)]

I will look into this, as always it greatly helps with these sort of bugs to have a reduced testcase!

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: Properly mark D2D DrawTarget changed

So, the rootcause of the issue is that a ClearRect operation is not considered 'changing' the surface, that causes snapshots' Copy-On-Write mechanism not to kick in, and you get contents as a source from after the surface was cleared.

This patch fixes the missing MarkChanged calls. As a workaround, you could consider doing a single pixel fillRect before your ClearRects on the source surface, then everything should work fine. Of course this has a small performance impact.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 564811 [details] [diff] [review]
Properly mark D2D DrawTarget changed

Yes, but add a test case please

Comment 4

[Bas Schouten (:bas.schouten)]

Attachment: Add reftest

Comment 5

[Bas Schouten (:bas.schouten)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7a6c01b22a1
https://hg.mozilla.org/integration/mozilla-inbound/rev/1aa4d9fa1bfb

Comment 6

[Bas Schouten (:bas.schouten)]

Comment on attachment 564811 [details] [diff] [review]
Properly mark D2D DrawTarget changed

Since this fix is very safe we should probably take it on Aurora.

I'm not 100% sure about beta since the bug isn't that bad and there's an easy workaround.

Comment 7

[David Woldrich]

I just took a look at the source for gfx/2d/DrawTargetD2D.cpp.  That is some fully badass looking code, big body of work there.  

Not fully understanding the code, just wanted to ask/verify that PrepareForDrawing should not be used in place of MarkChanged in ClearRect and CopySurface?  It seems like PrepareForDrawing is doing clipper management as well before making a call to MarkChanged?

If I run into any more issues, I'll try to put together a simple test before submitting a bug.  Thanks for looking into this one!

Comment 8

[Bas Schouten (:bas.schouten)]

(In reply to David Woldrich from comment #7)
> I just took a look at the source for gfx/2d/DrawTargetD2D.cpp.  That is some
> fully badass looking code, big body of work there.  
> 
> Not fully understanding the code, just wanted to ask/verify that
> PrepareForDrawing should not be used in place of MarkChanged in ClearRect
> and CopySurface?  It seems like PrepareForDrawing is doing clipper
> management as well before making a call to MarkChanged?
> 
> If I run into any more issues, I'll try to put together a simple test before
> submitting a bug.  Thanks for looking into this one!

The answer to your question is no :).

In Direct2D clearing is not supported when geometrical clips are popped, so we actually bypass the clipper altogether. For CopySurface Transform and Clip are  ignored as per API docs (see 2D.h).

Thanks for paying attention though!

Comment 9

[Ed Morley [:emorley]]

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

Comment 10

[Ed Morley [:emorley]]

And the reftest: https://hg.mozilla.org/mozilla-central/rev/1aa4d9fa1bfb

Comment 11

[LegNeato]

Comment on attachment 564811 [details] [diff] [review]
Properly mark D2D DrawTarget changed

We aren't seeing user reports of this generally so we'll wait for it to bubble up in the normal process.

Comment 12

[David Woldrich]

Bump.  The "hardware accelerated ClearRect not considered dirty" bug does not appear to be resolved in Firefox 9 on Windows 7.  Could this fix be a candidate for release in Firefox 10?  Thanks!

Comment 13

[Bas Schouten (:bas.schouten)]

(In reply to David Woldrich from comment #12)
> Bump.  The "hardware accelerated ClearRect not considered dirty" bug does
> not appear to be resolved in Firefox 9 on Windows 7.  Could this fix be a
> candidate for release in Firefox 10?  Thanks!

This fix is already in Firefox 10! So you should be seeing it there.

Comment 14

[David Woldrich]

Yay, thank you!  I'll go check out the beta now!