Closed
Bug 1070308
Opened 10 years ago
Closed 10 years ago
Add Acquire and Release semantics to SharedSurface
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: jgilbert, Assigned: jgilbert)
References
Details
Attachments
(1 file, 2 obsolete files)
6.89 KB,
patch
|
jgilbert
:
review+
|
Details | Diff | Splinter Review |
The real locking/fencing/waiting semantics we want are Acquire and Release for both Producer and Consumer. We need this architecture for using `SharedMutex`s with D3D11.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8492478 -
Flags: review?(jmuizelaar)
Comment 2•10 years ago
|
||
I had a super quick look at this and it looks like what we want.
Comment 3•10 years ago
|
||
Comment on attachment 8492478 [details] [diff] [review] recycle-fence Review of attachment 8492478 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/composite/TextureHost.cpp @@ +1166,5 @@ > + MOZ_ASSERT(!mIsLocked); > + > + EnsureTexSource(); > + > + mSurf->ConsumerAcquire(); It would work better if we did ConsumerAcquire before EnsureTexSource. I want to make it so that we only have to make it so that we only have to OpenShareHandle once per draw.
Attachment #8492478 -
Flags: review?(jmuizelaar) → review+
Comment 4•10 years ago
|
||
This patch seems to be hitting the assertion in ProducerRelease() called from GLScreenBuffer::Swap()
Comment 5•10 years ago
|
||
This fixes the assertion by calling Acquire in ::Resize and fixes another assertion by calling Acquire before EnsureTexSource.
Attachment #8492478 -
Attachment is obsolete: true
Attachment #8505651 -
Flags: review?(jgilbert)
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8505651 [details] [diff] [review] acquire-release.patch Review of attachment 8505651 [details] [diff] [review]: ----------------------------------------------------------------- Cool. It looks like we currently call Fence twice. This might explain some of the talos regressions. ::: gfx/gl/SharedSurface.h @@ +80,5 @@ > virtual void UnlockProdImpl() = 0; > > + virtual void ProducerAcquireImpl() {} > + virtual void ProducerReleaseImpl() { > + Fence(); This file is still 4-space.
Attachment #8505651 -
Flags: review?(jgilbert) → review+
Comment 7•10 years ago
|
||
Looks like we're still failing an assertion: https://tbpl.mozilla.org/?tree=Try&rev=002446aa01a2
Comment 8•10 years ago
|
||
This was without the double readback fix though. Trying again.
Comment 9•10 years ago
|
||
Still failing against current inbound: https://tbpl.mozilla.org/?tree=Try&rev=8a543c535e56
Comment 10•10 years ago
|
||
This adds a ProducerRelease() in Resize() that's called when needed.
Attachment #8505651 -
Attachment is obsolete: true
Attachment #8506378 -
Flags: review?(jgilbert)
Assignee | ||
Updated•10 years ago
|
Attachment #8506378 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e17d46ddda4
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6e17d46ddda4
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•