717921 - Making multiple ThebesSurfaces for the same DrawTarget confuses the Thebes surfaces

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

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]]

Attachment: Add UserData to DrawTarget

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: Make sure we only ever have one thebes surface

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Attachment: 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)]

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)]

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)]

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

Comment 7

[Matt Brubeck (:mbrubeck)]

Re-landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3b761d31434
https://hg.mozilla.org/integration/mozilla-inbound/rev/044c107a719f
https://hg.mozilla.org/integration/mozilla-inbound/rev/73f05b4552b0

Comment 8

[Joe Drew (not getting mail)]

https://hg.mozilla.org/mozilla-central/rev/73f05b4552b0
https://hg.mozilla.org/mozilla-central/rev/044c107a719f
https://hg.mozilla.org/mozilla-central/rev/d3b761d31434