Closed
Bug 1381393
Opened 7 years ago
Closed 7 years ago
Fix TextureClient assertion with OMTP
Categories
(Core :: Graphics: Layers, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: dvander, Assigned: dvander)
References
Details
Attachments
(1 file)
10.64 KB,
patch
|
mattwoodrow
:
review+
|
Details | Diff | Splinter Review |
When OMTP is enabled we lock TextureClients, record draw commands, then unlock them. This breaks an assertion that the borrowed DrawTarget is not used after unlocking [1]. This assertion is important in that doing so violates the API contract of Lock/Unlock. It's hard to fix the expected refcount since the ref could be added or dropped asynchronously. Looking at the API, it seems kind of pointless these days. It was designed for supporting different backends that might have various locking requirements, but (1) none of the backends we use have granular locks anymore, (2) we do not want more backends, ever, and (3) OMTP fundamentally breaks the way the API is supposed to work anyway, since we'd have to lock on one thread and unlock on another. I'd like to see if we can't rework how the locking/unlocking happens just enough to bypass all the non-OMTP code. If it looks too difficult we can just flag the TC (maybe via OpenMode) to silence the assertion. [1] http://searchfox.org/mozilla-central/rev/01d27fdd3946f7210da91b18fcccca01d7324fe2/gfx/layers/client/TextureClient.cpp#561
Assignee | ||
Comment 1•7 years ago
|
||
All the classes involved here are super intertwined, so this patch just tries to bypass the assertion when async painting.
Attachment #8888647 -
Flags: review?(matt.woodrow)
Comment 2•7 years ago
|
||
Comment on attachment 8888647 [details] [diff] [review] patch Review of attachment 8888647 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/CompositorTypes.h @@ +256,5 @@ > + > + // This is only used in conjunction with OMTP to indicate that the DrawTarget > + // that is being borrowed will be painted asynchronously, and so will outlive > + // the write lock. > + OPEN_ASYNC_WRITE = 0x08 Can just use 0x4 here?
Attachment #8888647 -
Flags: review?(matt.woodrow) → review+
Assignee | ||
Comment 3•7 years ago
|
||
Yup, will fix that.
Pushed by danderson@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4cd3ff1e99a2 Don't assert about borrowed DT refcounts when async painting. (bug 1381393, r=mattwoodrow)
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4cd3ff1e99a2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•