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.
Summary: [Chromium, Skia] Add virtual to WebGraphicsContext3D to create a new GrGLInte...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brian Salomon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-01 06:39 PDT by Brian Salomon
Modified: 2011-09-01 16:24 PDT (History)
5 users (show)

See Also:


Attachments
Patch (6.42 KB, patch)
2011-09-01 06:41 PDT, Brian Salomon
no flags Details | Formatted Diff | Diff
Patch (6.30 KB, patch)
2011-09-01 13:19 PDT, Brian Salomon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Salomon 2011-09-01 06:39:12 PDT
[Chromium, Skia] Add virtual to WebGraphicsContext3D to create a new GrGLInterface per context. Insert per-skia-GL-function callback to set correct GL context.
Comment 1 Brian Salomon 2011-09-01 06:41:09 PDT
Created attachment 105952 [details]
Patch
Comment 2 Brian Salomon 2011-09-01 06:50:04 PDT
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.
Comment 3 Kenneth Russell 2011-09-01 12:14:23 PDT
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.
Comment 4 Brian Salomon 2011-09-01 13:19:58 PDT
Created attachment 106016 [details]
Patch
Comment 5 Brian Salomon 2011-09-01 13:21:54 PDT
(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.
Comment 6 Kenneth Russell 2011-09-01 14:54:51 PDT
Comment on attachment 106016 [details]
Patch

Thanks. r=me
Comment 7 WebKit Review Bot 2011-09-01 16:24:09 PDT
Comment on attachment 106016 [details]
Patch

Clearing flags on attachment: 106016

Committed r94359: <http://trac.webkit.org/changeset/94359>
Comment 8 WebKit Review Bot 2011-09-01 16:24:14 PDT
All reviewed patches have been landed.  Closing bug.