Making multiple ThebesSurfaces for the same DrawTarget confuses the Thebes surfaces

RESOLVED FIXED in mozilla12

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

unspecified
mozilla12
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 588408 [details] [diff] [review]
Add UserData to DrawTarget
Attachment #588408 - Flags: review?(bas.schouten)
(Assignee)

Comment 2

6 years ago
Created attachment 588670 [details] [diff] [review]
Make sure we only ever have one thebes surface
Attachment #588670 - Flags: review?(bas.schouten)
(Assignee)

Comment 3

6 years ago
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.
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):
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
Assignee: nobody → jmuizelaar
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
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/73f05b4552b0
https://hg.mozilla.org/mozilla-central/rev/044c107a719f
https://hg.mozilla.org/mozilla-central/rev/d3b761d31434
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Depends on: 720299
You need to log in before you can comment on or make changes to this bug.