Summary: | [chromium] GLES2Context is in platform/ and shouldn't depend on Page | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Robinson <jamesr> | ||||||
Component: | New Bugs | Assignee: | James Robinson <jamesr> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, eric, fishd, jamesr, kbr, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
James Robinson
2010-07-13 14:43:49 PDT
Created attachment 61428 [details]
Patch
My plan for creating GLES2Contexts from WebCore is to have two functions on ChromeClient: // Returns a GLES2Context used for rendering directly to the screen PassOwnPtr<GLES2Context> getOnscreenGLES2Context(); // Returns a GLES2Context for rendering offscreen that is capable of sharing // resources with the compositor's context PassOwnPtr<GLES2Context> getOffscreenGLES2Context(); The lifetime of GLES2Contexts is still kind of awkward. Making two calls to getOnscreenGLES2Context() will return two OwnPtr<>s that each wrap the same underlying object, but deleting a GLES2Context does not destroy the actual underlying object. Calling GLES2Context::destroy() does do something destructive. I think it's still valid to have this be an OwnPtr<> to make sure we don't leak the wrapper object (which I believe we do currently). I can post that as a followup or as a separate patch. Adding stuff to ChromeClient involves a loooot of plumbing so it's a rather noisy patch. Created attachment 61581 [details]
Patch
This patch adds in the onscreen/offscreen getters to ChromeClientChromium. They are currently uncalled, but that won't be true for long. One unfortunate thing is that this exposes two types of WebCore::GLES2Context's - those that own their underlying WebKit::WebGLES2Context and those that don't. When getting the GLES2Context for compositing, the caller doesn't want to have ownership. When getting an offscreen context the caller does want ownership. I added a flag to the GLES2ContextInternal constructor to allow for both kinds but it's kind of ugly. We don't need the ChromeClientChromium::getOnscreenGLES2Context() function yet so we could delete it for now and figure it out later. Comment on attachment 61581 [details]
Patch
WebKit/chromium/public/WebGLES2Context.h:49
+ WEBKIT_API virtual bool initialize(WebView*, WebGLES2Context* parent) = 0;
no need for WEBKIT_API on virtual methods. no need to export
these from WebKit when it is built as a DLL.
WebKit/chromium/src/GLES2ContextInternal.h:55
+ }
} // namespace WebCore
WebKit/chromium/src/WebViewImpl.cpp:2139
+ return WebCore::GLES2Context::create(WebCore::GLES2ContextInternal::create(gles2Context(), false));
no need for the WebCore:: prefix in this .cpp file given the
using declaration at the top of the file.
WebKit/chromium/src/WebViewImpl.cpp:2142
+ PassOwnPtr<WebCore::GLES2Context> WebViewImpl::getOffscreenGLES2Context()
ditto about the WebCore:: prefix
otherwise, R=me with these nits addressed.
Committed r63383: <http://trac.webkit.org/changeset/63383> http://trac.webkit.org/changeset/63383 might have broken Chromium Mac Release The following changes are on the blame list: http://trac.webkit.org/changeset/63384 http://trac.webkit.org/changeset/63385 http://trac.webkit.org/changeset/63386 http://trac.webkit.org/changeset/63389 http://trac.webkit.org/changeset/63383 Fixed the compile error in http://trac.webkit.org/changeset/63392 |