Closed Bug 717921 Opened 10 years ago Closed 10 years ago

Making multiple ThebesSurfaces for the same DrawTarget confuses the Thebes surfaces


(Core :: Graphics, defect)

Not set





(Reporter: jrmuizel, Assigned: jrmuizel)




(3 files)

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.
Attachment #588408 - Flags: review?(bas.schouten)
Attachment #588670 - Flags: review?(bas.schouten)
This makes sure that we Mark the thebes surface associated with a draw target dirty.
Attachment #589007 - Flags: review?(bas.schouten)
Attachment #589007 - Flags: review?(bas.schouten) → review+
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.
Attachment #588408 - Flags: review?(bas.schouten) → review+
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.
Attachment #588670 - Flags: review?(bas.schouten) → review+
Landed on inbound but backed out (along with the other patches it landed with):

because one of the changes caused reftests to fail at startup with "REFTEST TEST-UNEXPECTED-FAIL | | EXCEPTION: SyntaxError: missing ) in parenthetical":
Assignee: nobody → jmuizelaar
Depends on: 720299
You need to log in before you can comment on or make changes to this bug.