Closed
Bug 1060085
Opened 8 years ago
Closed 8 years ago
Add Thread-local versions of ShSurf::Fence/Poll/Wait
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: jgilbert, Assigned: jgilbert)
References
Details
Attachments
(2 files, 2 obsolete files)
10.64 KB,
patch
|
u480271
:
review+
|
Details | Diff | Splinter Review |
4.73 KB,
patch
|
jgilbert
:
review+
|
Details | Diff | Splinter Review |
We get access to some better synchronization opportunities if we have thread-local versions of ShSurf::Poll and Wait. For instance, if we add a frame of delay, but call poll/wait on the content thread, we can wait until then to readback, which may allow for better throughput than doing readback during Fence(). Also, NV_fence for ANGLE has to happen on the same GLContext, so we can only use fTestFence and fFinishFence on the same thread we make the fence on.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8480887 -
Flags: review?(dglastonbury)
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8480891 -
Flags: review?(dglastonbury)
Assignee | ||
Comment 3•8 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=8ce046a47fdc
Comment on attachment 8480887 [details] [diff] [review] patch 1: Add entrypoints and add NV_fence support to ANGLE ShSurfs Review of attachment 8480887 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/SharedSurface.h @@ +86,5 @@ > > + // Use these if you can. They can only be called from the Content > + // thread, though! > + virtual void Fence_ThreadLocal() { > + Fence(); Should we assert the thread it's being called from? I'm not a huge fan of _ThreadLocal(). Maybe _ContentThread, or something. I don't know, but I also don't think it warrants an r-.
Attachment #8480887 -
Flags: review?(dglastonbury) → review+
Comment on attachment 8480891 [details] [diff] [review] patch 2: Add thread-local Fence/Poll/Wait to Basic ShSurf Review of attachment 8480891 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/SharedSurfaceGL.cpp @@ +72,5 @@ > , mTex(tex) > , mFB(0) > + , mIsDataCurrent(false) > +#ifdef DEBUG > + , mParentThread(NS_GetCurrentThread()) Should this be mOwningThread, perhaps? Parent implies hierarchy to me (object-to-object). In this case, I think a thread "owns" the fence/surf/etc.
Attachment #8480891 -
Flags: review?(dglastonbury) → review+
Assignee | ||
Comment 6•8 years ago
|
||
I changed the name and made it assert without having to add asserts to each implementation.
Attachment #8480887 -
Attachment is obsolete: true
Attachment #8481575 -
Flags: review?(dglastonbury)
Assignee | ||
Comment 7•8 years ago
|
||
This is just a de-bitrotted version of the previous r+. r=kamidphish
Attachment #8480891 -
Attachment is obsolete: true
Attachment #8481576 -
Flags: review+
Attachment #8481575 -
Flags: review?(dglastonbury) → review+
Assignee | ||
Comment 8•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1bc9a5c17ff https://hg.mozilla.org/integration/mozilla-inbound/rev/9d4fc9f2d5d1
OS: Windows 8.1 → All
Hardware: x86_64 → All
Comment 9•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d1bc9a5c17ff https://hg.mozilla.org/mozilla-central/rev/9d4fc9f2d5d1
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•