See bug 525673 -- the testcase there uses "canvas.width = canvas.width;" at the start of its draw loop as a way to clear/reset the canvas, becuase the canvas state and contents are supposed to be reset whenever width or height are set. We always create a new surface in response to this, even when the width/height are the same as the previous; we should instead just clear the surface, but still create a new gfxContext to avoid the memory churn. Would be good to profile this to make sure that it's a win, but I think it will be.
8 years ago
Whiteboard: [good first bug]
Created attachment 414436 [details] [diff] [review] Patch Reuses the code in InitializeWithSurface to create a new gfxContext and initialize the canvas default state.
Assignee: nobody → wesongathedeveloper
Status: NEW → ASSIGNED
Attachment #414436 - Flags: review?(vladimir)
With the attached perf test, average times were approx 183ms without the patch and 122ms with the patch.
Comment on attachment 414436 [details] [diff] [review] Patch missed this, sorry! Catching up on review backlog this week.
Attachment #414436 - Flags: review?(vladimir) → review+
8 years ago
Whiteboard: [good first bug] → [has patch]
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [has patch]
Target Milestone: --- → mozilla1.9.3a1
Backed out, it seems to cause a mochitest failure in content/canvas/test/test_canvas.html: ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | unexpected exception thrown in: test_2d_path_arcTo_scale http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263467375.1263467986.25860.gz I'm also seeing a bunch of "unexpected exception thrown" failures in my local OS X debug build that go away when I remove this patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
See also http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263470418.1263471071.27256.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263471234.1263472035.6214.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263469753.1263471573.761.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263473180.1263474919.7568.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263472453.1263473649.25234.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263474445.1263475246.11881.gz
I've just applied this patch to a new build on Windows 7 and all the tests pass. looking into this to see what's going on...
Created attachment 434128 [details] Test results This patch does not cause any test failures on my XP installation either. How long are the build logs kept for? I wasn't aware that they expire that soon... Any way to rebuild this on a try server or smth for new logs? (I don't have access to a Mac, which is where I think this was problem).
This patch has rotted sadly (sorry for letting it sit this long). The problem is most liking the skipped call to Destroy() (now called Reset()) which did more than just release the surface. I'd suggest using an nsRefPtr to keep the surface alive, and still call Reset(). If you get a fixed patch uploaded, I'd be happy to handle running it on tryserver and landing it for you.
You need to log in before you can comment on or make changes to this bug.