This would make WebGL behave more like the media layer. It would not only clean up the code, but would make it possible to get rid of one GL context and a copy of the pixels. We may also want to rename Canvas3DLayer to WebGLLayer.
I will do this in 3 phases: 1) Rename GraphicsContext3DMac.cpp to GraphicsContext3DMac.mm to allow handling of CALayer 2) Rename Canvas3DLayer to WebGLLayer 3) Move WebGLLayer ownership to GraphicsContext3D and use a single CGLContext for rendering
Created attachment 58841 [details] Patch to rename GraphicsContext3DMac.cpp
Attachment 58841 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/3324197
Comment on attachment 58841 [details] Patch to rename GraphicsContext3DMac.cpp r=me assuming that you also intend to fix the Xcode project.
Landed GraphicsContext3DMac rename in http://trac.webkit.org/changeset/61262 (along with xcodeproj change)
Created attachment 58901 [details] Patch to rename Canvas3DLayer to WebGLLayer
I'd suggesting using a more specific name than WebGLLayer, to avoid Obj-C name conflicts with embedding apps (Obj-C just has one global namespace). I have to rename WebLayer/WebTiledLayer for the same reasons.
Comment on attachment 58841 [details] Patch to rename GraphicsContext3DMac.cpp Cleared Simon Fraser's review+ from obsolete attachment 58841 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Landed WebGLLayer rename in http://trac.webkit.org/changeset/61263
Created attachment 59413 [details] Final patch
Attachment 59413 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/platform/graphics/mac/GraphicsLayerCA.h:336: Missing space after , [whitespace/comma] [3] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 59413 [details] Final patch > Index: WebCore/platform/graphics/mac/GraphicsContext3DMac.mm > =================================================================== > +#ifndef NDEBUG > + [m_webGLLayer.get() setName:@"3D Layer"]; > +#endif I think "WebGL Layer" would be a better name. > Index: WebCore/platform/graphics/mac/GraphicsLayerCA.h > =================================================================== > #if ENABLE(3D_CANVAS) > - virtual void setGraphicsContext3DNeedsDisplay(); > + virtual void setWebGLNeedsDisplay(); > #endif Do you really need the setWebGLNeedsDisplay() method? The GraphicsContext3D now holds onto the layer, so maybe it should do the -setNeedsDisplay itself? However, this will bypass the syncing logic, so may cause flashes with some content. > Index: WebCore/platform/graphics/mac/GraphicsLayerCA.mm > =================================================================== > -void GraphicsLayerCA::setGraphicsContext3DNeedsDisplay() > +void GraphicsLayerCA::setWebGLNeedsDisplay() > { > - if (m_contentsLayerPurpose == ContentsLayerForGraphicsLayer3D) > + if (m_contentsLayerPurpose == ContentsLayerForWebGL) > [m_contentsLayer.get() setNeedsDisplay]; > } This may cause an early CA commit, which can result in flashes in some content. You should really use a dirty bit for this. r=me but please reconsider setWebGLNeedsDisplay().
Created attachment 59418 [details] replacement patch with setContentsNeedsDisplay
Attachment 59418 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/platform/graphics/mac/GraphicsLayerCA.h:335: Missing space after , [whitespace/comma] [3] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 59418 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3280610
Created attachment 59419 [details] Replacement patch fixing style issue and Qt build failure
Landed final patch in http://trac.webkit.org/changeset/61631
The changes to RenderLayerBacking.cpp break the Chromium build (and most likely the Qt build as well). [line: 219] if (context->graphicsContext3D()->platformLayer()) platformLayer() is only defined for PLATFORM(MAC)
I suppose the bots didn't catch this because they build with 3D_CANVAS off? Are either Qt or Chromium building with both 3D_CANVAS and ACCELERATED_COMPOSITING enabled? I didn't think so. Vangelis, since you have a Chromium build environment, could you make a #else case that defines platformLayer() for other than Mac. I suppose it would just be a stub for now.
chromium on windows currently builds with both ACCELERATED_COMPOSITING and 3D_CANVAS but it look like the windows trybot didn't get to run. I'll check in a quick fix to get the chrome build green again, however, there's an issue with the new API. The idea that GraphicsContext3D knows about the PlatformLayer it renders into is, I believe, unique to CoreAnimation. In other ports, the PlatformLayer's actually get created by the GraphicsLayer's and are managed by them as well. I found the old API where you provide a pointer to the GraphicsContext3D when you set the contents of a layer more general. Can't you extract the CALayer information from it within GraphicsLayerCA::setContentsToWebGL ?
(In reply to comment #20) > chromium on windows currently builds with both ACCELERATED_COMPOSITING and 3D_CANVAS but it look like the windows trybot didn't get to run. > > I'll check in a quick fix to get the chrome build green again, however, there's an issue with the new API. The idea that GraphicsContext3D knows about the PlatformLayer it renders into is, I believe, unique to CoreAnimation. In other ports, the PlatformLayer's actually get created by the GraphicsLayer's and are managed by them as well. I found the old API where you provide a pointer to the GraphicsContext3D when you set the contents of a layer more general. Can't you extract the CALayer information from it within GraphicsLayerCA::setContentsToWebGL ? One of the biggest motivators for this change was to make WebGL layers behave like the media layer, which hands ownership of the layer to the client. For video this allows acceleration of the video contents without GraphicsLayer having to know anything about it. The same is true of WebGL layers. There was hackery involved to communicate enough information to GraphicsLayer to get it to be able to render accelerated 3D content. With the current design GraphicsLayer doesn't have to know about GraphicsContext3D or WebGLLayer. So I think this is the better design. I think your design would really benefit from contracting out the creation and management of the PlatformLayer. Remember PlatformLayer could be anything you want in Chromium, a GL context, a GraphicsContext3D, or a wrapper with the appropriate functionality. It really does reduce dependencies.
*** Bug 34036 has been marked as a duplicate of this bug. ***