Closed Bug 1060085 Opened 8 years ago Closed 8 years ago

Add Thread-local versions of ShSurf::Fence/Poll/Wait


(Core :: Graphics: CanvasWebGL, defect)

Not set





(Reporter: jgilbert, Assigned: jgilbert)




(2 files, 2 obsolete files)

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.
Blocks: 1052234
Blocks: 1054808
Attachment #8480891 - Flags: review?(dglastonbury)
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+
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)
This is just a de-bitrotted version of the previous r+.
Attachment #8480891 - Attachment is obsolete: true
Attachment #8481576 - Flags: review+
Attachment #8481575 - Flags: review?(dglastonbury) → review+
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.