Bug 67395 – [Chromium, Skia] Add virtual to WebGraphicsContext3D to create a new GrGLInterface per context. Insert per-skia-GL-function callback to set correct GL context.

• Component: Platform
• Status: RESOLVED FIXED

[Brian Salomon]

[Chromium, Skia] Add virtual to WebGraphicsContext3D to create a new GrGLInterface per context. Insert per-skia-GL-function callback to set correct GL context.

[Brian Salomon]

Created attachment 105952 [details]
Patch

[Brian Salomon]

The idea behind this change is to create a new GrGLInterface per-WG3D and to use a new callback facility in Ganesh to make sure the correct context is set before each GL call. The motivation is that we keep seeing wrong context bugs.

The sister change to this in Chrome is here:
http://codereview.chromium.org/7756004/

After both changes have landed I can remove PlatformContextSkia::makeGrContextCurrent() and all callsites as well as the WGC3D::grGLInterface virtual.

[Kenneth Russell]

Comment on attachment 105952 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105952&action=review

Sorry, but I have to r- this because the license needs to be updated in the new file. Also CC'ing fishd as he should review all WebKit API changes. It looks OK overall to me though.

> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:166
> +        // remove this block after WGC3D subclasses in Chromium overriding using grGLInterface()

Add FIXME and please make this comment and the one below a complete sentence.

> Source/WebKit/chromium/src/WebGraphicsContext3D.cpp:15
> + *     from this software without specific prior written permission.

Sorry, this is the wrong version of the license file. I know there are a ton of files that have this license but we should avoid adding more that are wrong. Please derive the license text from Source/WebKit/LICENSE, or see e.g. Source/WebKit/chromium/tests/PODIntervalTreeTest.cpp.

[Brian Salomon]

Created attachment 106016 [details]
Patch

[Brian Salomon]

(In reply to comment #3)
> (From update of attachment 105952 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105952&action=review
> 
> Sorry, but I have to r- this because the license needs to be updated in the new file. Also CC'ing fishd as he should review all WebKit API changes. It looks OK overall to me though.
> 
> > Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:166
> > +        // remove this block after WGC3D subclasses in Chromium overriding using grGLInterface()
> 
> Add FIXME and please make this comment and the one below a complete sentence.
> 
> > Source/WebKit/chromium/src/WebGraphicsContext3D.cpp:15
> > + *     from this software without specific prior written permission.
> 
> Sorry, this is the wrong version of the license file. I know there are a ton of files that have this license but we should avoid adding more that are wrong. Please derive the license text from Source/WebKit/LICENSE, or see e.g. Source/WebKit/chromium/tests/PODIntervalTreeTest.cpp.

The latest patch addresses the license and comments.

[Kenneth Russell]

Comment on attachment 106016 [details]
Patch

Thanks. r=me

[WebKit Review Bot]

Comment on attachment 106016 [details]
Patch

Clearing flags on attachment: 106016

Committed r94359: <http://trac.webkit.org/changeset/94359>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.