Last Comment Bug 691571 - Canvas 2D hardware acceleration makes createPattern flicker
: Canvas 2D hardware acceleration makes createPattern flicker
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: 7 Branch
: x86_64 Windows 7
: -- normal (vote)
: mozilla10
Assigned To: Bas Schouten (:bas.schouten)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-03 16:20 PDT by David Woldrich
Modified: 2011-12-21 10:06 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
flickeryrace.zip (1.37 MB, application/octet-stream)
2011-10-03 16:20 PDT, David Woldrich
no flags Details
Properly mark D2D DrawTarget changed (1.38 KB, patch)
2011-10-05 06:04 PDT, Bas Schouten (:bas.schouten)
jmuizelaar: review+
christian: approval‑mozilla‑aurora-
christian: approval‑mozilla‑beta-
Details | Diff | Splinter Review
Add reftest (2.12 KB, patch)
2011-10-05 07:04 PDT, Bas Schouten (:bas.schouten)
jmuizelaar: review+
Details | Diff | Splinter Review

Description David Woldrich 2011-10-03 16:20:46 PDT
Created attachment 564385 [details]
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) 2011-10-05 04:32:42 PDT
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) 2011-10-05 06:04:18 PDT
Created attachment 564811 [details] [diff] [review]
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] 2011-10-05 06:17:37 PDT
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) 2011-10-05 07:04:17 PDT
Created attachment 564834 [details] [diff] [review]
Add reftest
Comment 6 Bas Schouten (:bas.schouten) 2011-10-05 07:12:46 PDT
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 2011-10-05 23:48:06 PDT
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) 2011-10-06 03:30:15 PDT
(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] 2011-10-06 03:51:06 PDT
https://hg.mozilla.org/mozilla-central/rev/f7a6c01b22a1
Comment 10 Ed Morley [:emorley] 2011-10-06 03:51:58 PDT
And the reftest: https://hg.mozilla.org/mozilla-central/rev/1aa4d9fa1bfb
Comment 11 christian 2011-10-06 14:54:31 PDT
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 2011-12-20 17:02:36 PST
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) 2011-12-20 18:07:57 PST
(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 2011-12-21 10:06:32 PST
Yay, thank you!  I'll go check out the beta now!

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