Proposing new CHROMIUM_sync_point API.

gpu/GLES2/extensions/CHROMIUM/CHROMIUM_sync_point.txt

Index: gpu/GLES2/extensions/CHROMIUM/CHROMIUM_sync_point.txt
diff --git a/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_sync_point.txt b/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_sync_point.txt
index 965cb44b0c066306163f84404232be632a42f644..08cf38777fca29d9988570986a9b3bafb893d946 100644
--- a/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_sync_point.txt
+++ b/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_sync_point.txt
@@ -8,7 +8,7 @@ Name Strings
 
 Version
 
-    Last Modifed Date: February 25, 2013
+    Last Modifed Date: September 8, 2015
 
 Dependencies
 
@@ -34,35 +34,60 @@ New Tokens
 
     None
 
+New Types
+
+    typedef struct __CHROMIUMsync *CHROMIUMsync;

[jbauman, 2015/09/08 20:54:13]

What exactly is __CHROMIUMsync? Can it be passed via IPC?

[David Yen, 2015/09/08 21:35:29]

In order to make the tracking of lifetimes easier, I have changed this to use
something similar to how mailbox's are implemented. It makes it easier to pass
it via IPC as well.

+
 New Procedures and Functions
 
     The command
 
         uint InsertSyncPointCHROMIUM()
 
-    flushes the stream of commands for the current context and creates and
-    inserts a sync point. The sync point acts as a fence, which is signaled when
-    previous commands have been submitted to the server, or when the context is
-    destroyed, whichever happens first. The sync point name is returned. The
-    sync point is implicitly deleted when it becomes signaled. The sync point
-    namespace is shared between all contexts on the same server, including other
-    context groups.
+    inserts a sync point in the current command stream. The sync point acts as
+    a fence, which is signaled when previous commands have been submitted to
+    the server, or when the context is destroyed, whichever happens first.
+    The sync point name for the current context is returned. Before the
+    returned sync point is waited upon, the command must be flushed and
+    converted to a CHROMIUMsync object using GenSyncPointCHROMIUM. The
+    CHROMIUMsync object returned from GenSyncPointCHROMIUM is shared between
+    all contexts.
+
+    The command
+
+        CHROMIUMsync GenSyncPointCHROMIUM(uint sync_point)
 
+    converts a sync point name for the current context to a sync point object
+    which can be waited on and shared between all contexts on the same server.
+    <sync_point> must be flushed before this function may be called, otherwise
+    an INVALID_OPERATION error is generated.

[piman, 2015/09/08 18:21:01]

Can you clarify whether the CHROMIUMsync handle namespace is per-context,
per-share-group, per-client or global across all clients?

If one of the first 2, one would expect the CHROMIUMsync handle is deleted when
the context/share-group (respectively) is destroyed, like other similar names.
What about the later 2?

[David Yen, 2015/09/08 19:07:53]

Improved documentation on this. It is shared globally, but I have not decided
how the lifetime would be if the context was destroyed. It would probably be
well behaved if we just deleted the object on destruction of the context, but
technically waits do not care if the context is destroyed or not. If the other
contexts can get the gpu_channel id, routing id, and sync point id we can still
wait on it as long as all the commands are queued before it was destroyed. To
have that working though, we would need some way to ref count the object and
track who has the handle. I'm not sure if I want to do all of that.

Another simple solution would be if we just replaced the sync point object and
returned those 3 numbers directly. The client can then IPC the 3 integers to
other contexts without worrying about any lifetimes, but that may be leaking too
much implementation details and also does not give us the flexibility of
expanding the object later. Although I can't think of a reason why we would need
anymore than those 3, even streams do not add any extra necessary information.
Another downside is that it would also make it so they don't need to go through
GenSyncPointCHROMIUM (which verifies flushes are done correctly), since the gpu
channel and routing id are always the same.

 
     The command
 
-        void WaitSyncPointCHROMIUM(uint sync_point)
+        void WaitSyncPointCHROMIUM(CHROMIUMsync sync_point)
 
     causes the current context to stop submitting commands until the specified
     sync point becomes signaled. This is implemented as a server-side wait.
     <sync_point> is the name of the sync point to wait for. If <sync_point>
-    isn't a valid sync point returned by InsertSyncPointCHROMIUM, or if the sync
+    isn't a valid sync point returned by GenSyncPointCHROMIUM, or if the sync
     point has already been deleted, the command is equivalent to a no-op and no
     error is generated.
 
+    The command
+
+        void DeleteSyncPointCHROMIUM(CHROMIUMsync sync_point)
+
+    deletes the corresponding sync point object and makes it so no context can
+    wait on <sync_point> anymore. This will not affect any WaitSyncPointCHROMIUM
+    commands that are already blocking on <sync_point>.

[piman, 2015/09/08 18:21:01]

I'm not sure what "already blocking on <sync_point>" means. That wait happens
server-side, on another context/stream, so before/after is not well-defined.

[David Yen, 2015/09/08 19:07:53]

I wanted to communicate that on the client side the commands do not rely on the
CHROMIUMsync object to exist before it is flushed to the server. As long as the
WaitSyncPointCHROMIUM is called and it extracts out the necessary information
(gpu channel, routing id, sync point number) the sync point object can be
deleted.

I've tried to improve the documentation about this more.

+
 Errors
 
-    None.
+    INVALID_VALUE is generated if the <sync_point> parameter of
+    GenSyncPointCHROMIUM is not a valid local sync point name.
+
+    INVALID_OPERATION is generated if the <sync_point> parameter of
+    GenSyncPointCHROMIUM has not been flushed.
 
 New State
 
@@ -71,3 +96,6 @@ New State
 Revision History
 
     2/25/2013    Documented the extension
+
+    9/8/2015     Added CHROMIUMsync, WaitSyncPointCHROMIUM,
+    DeleteSyncPointCHROMIUM