Bug 84473 – [chromium] GraphicsContext3D should ask WebGraphicsContext3D for unsupported extensions

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit Misc.

Currently, GC3D::supportsExtension returns true if the extension is in the list obtained from gl, or if the extension is requestable from the WebGraphicsContext3D. We should return false if the extension is explicitly unsupported by the WebGraphicsContext3D.

Created attachment 138132 [details]
Patch

[James Robinson]

(In reply to comment #0)
> if the extension is explicitly unsupported by the WebGraphicsContext3D.

What does this mean?  Is there a distinction between unsupported and not supported?

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

(In reply to comment #2)
> (In reply to comment #0)
> > if the extension is explicitly unsupported by the WebGraphicsContext3D.
> 
> What does this mean?  Is there a distinction between unsupported and not supported?

WebGraphicsContext3DInProcessCommandBuffer does not support swap acks. It doesn't make sense in its context. Unfortunately GC3D::supportsExtension(swap acks) return true. GC3D only asks its WGC3D for a white list of requestable extensions. The logic is something like:

supportsExtension(ext):
 return supportedByGL(ext) || isRequestableFromWGC3D(ext);

So there's no way for the WGC3D to veto an extension. The intent of this patch is to allow WGC3D to return a blacklist so that we can do something like

supportsExtension(ext):
 return (supportedByGL(ext) || isRequestableFromWGC3D(ext)) && !isUnsupportedByWG3D(ext);

[Nat Duca]

Comment on attachment 138132 [details]
Patch

Does it make more sense to just make the wgc3dcbi/ipcbi implementation of supportsExtensnion() return false in this case? That seems cleaner to me than ading another wgc3d interface.

(In reply to comment #5)
> (From update of attachment 138132 [details])
> Does it make more sense to just make the wgc3dcbi/ipcbi implementation of supportsExtensnion() return false in this case? That seems cleaner to me than ading another wgc3d interface.

AFAIK, wgc3d* does not implement supportsExtension. If it did, and it were called, then I agree that wgc3d*::supportsExtension would be the ideal place to do this check.

It looks like the extensions object's supportExtension(ext) turns around and calls GC3D::supportsExtension(ext), but the GC3D does not consult its WGC3D. It just checks its two lists and returns true if it's in either. One list that the GC3D checks is a whitelist from the WGC3D. It seemed in keeping with the current code for the WGC3D to also provide a blacklist.

[Nat Duca]

But how does it get the list? Can't we just precondition the list it gets to not have asyncswap? It seems like the core issue here is its lying when it supports the extension.

(In reply to comment #7)
> But how does it get the list? Can't we just precondition the list it gets to not have asyncswap? It seems like the core issue here is its lying when it supports the extension.

Ah, yes we can. It turns out that _both_ lists used by GC3D pass through the WGC3D -- I had thought it was only one. Will close this issue.

[Eric Seidel (no email)]

Comment on attachment 138132 [details]
Patch

Cleared review? from attachment 138132 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).