Last Comment Bug 717921 - Making multiple ThebesSurfaces for the same DrawTarget confuses the Thebes surfaces
: Making multiple ThebesSurfaces for the same DrawTarget confuses the Thebes su...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla12
Assigned To: Jeff Muizelaar [:jrmuizel]
:
Mentors:
Depends on: 720299
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-13 07:01 PST by Jeff Muizelaar [:jrmuizel]
Modified: 2012-01-22 21:45 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add UserData to DrawTarget (4.64 KB, patch)
2012-01-13 07:03 PST, Jeff Muizelaar [:jrmuizel]
bas: review+
Details | Diff | Splinter Review
Make sure we only ever have one thebes surface (5.65 KB, patch)
2012-01-14 11:49 PST, Jeff Muizelaar [:jrmuizel]
bas: review+
Details | Diff | Splinter Review
Always get a thebes surface to mark dirty (1.22 KB, patch)
2012-01-16 14:17 PST, Jeff Muizelaar [:jrmuizel]
bas: review+
Details | Diff | Splinter Review

Description Jeff Muizelaar [:jrmuizel] 2012-01-13 07:01:49 PST
Thebes surfaces expect to be MarkedDirty when they are drawn to by another mechanism. In my case I have two Thebes surfaces drawing to the same CGContext. This confuses each of them because they are not marking each other dirty when they modify the CGContext.

I plan on fixing this by add user data to the DrawTarget and hanging the Thebes surface off of that.
Comment 1 Jeff Muizelaar [:jrmuizel] 2012-01-13 07:03:15 PST
Created attachment 588408 [details] [diff] [review]
Add UserData to DrawTarget
Comment 2 Jeff Muizelaar [:jrmuizel] 2012-01-14 11:49:52 PST
Created attachment 588670 [details] [diff] [review]
Make sure we only ever have one thebes surface
Comment 3 Jeff Muizelaar [:jrmuizel] 2012-01-16 14:17:37 PST
Created attachment 589007 [details] [diff] [review]
Always get a thebes surface to mark dirty

This makes sure that we Mark the thebes surface associated with a draw target dirty.
Comment 4 Bas Schouten (:bas.schouten) 2012-01-16 15:23:27 PST
Comment on attachment 588408 [details] [diff] [review]
Add UserData to DrawTarget

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

::: gfx/2d/UserData.h
@@ +58,5 @@
> +    // We could keep entries in a std::vector instead of managing it by hand
> +    // but that would propagate an stl dependency out which we'd rather not
> +    // do (see bug 666609). Plus, the entries array is expect to stay small
> +    // so doing a realloc everytime we add a new entry shouldn't be too costly
> +    entries = static_cast<Entry*>(realloc(entries, sizeof(Entry)*(count+1)));

This realloc might be fallible, let's check that.

We also probably want to add a method to clear user data. We can do that in a follow-up.
Comment 5 Bas Schouten (:bas.schouten) 2012-01-16 15:26:35 PST
Comment on attachment 588670 [details] [diff] [review]
Make sure we only ever have one thebes surface

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

It might be worth it to clear cairo surfaces associated with a drawtarget. The strong reference means for example for D2D that cairo's scratch surface will be kept alive (well after a user being done) and consume extra VRAM. We can deal with this in a follow-up though.
Comment 6 Matt Brubeck (:mbrubeck) 2012-01-17 10:16:31 PST
Landed on inbound but backed out (along with the other patches it landed with):
https://hg.mozilla.org/integration/mozilla-inbound/rev/52f52eb53549

because one of the changes caused reftests to fail at startup with "REFTEST TEST-UNEXPECTED-FAIL | | EXCEPTION: SyntaxError: missing ) in parenthetical":
https://tbpl.mozilla.org/php/getParsedLog.php?id=8612031&tree=Mozilla-Inbound

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