Some code like GraphicsContext3D is used outside of WebGL, e.g. for accelerated canvas and TextureMapper. It should be guarded with a USE flag, maybe USE(GRAPHICS_CONTEXT_3D), and be turned on when any of the features requiring it is enabled.
Sounds reasonable.
Created attachment 150849 [details] Patch
I'm using USE(3D_GRAPHICS) as a guard, but I don't mind using something else like HAVE(OPENGL) or USE(GRAPHICS_CONTEXT_3D), whatever can get a concensus and is not tied to ENABLE(WEBGL).
Comment on attachment 150849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150849&action=review Nice! > Source/WebCore/WebCore.pri:211 > +haveQt(5)|contains(QT_CONFIG, opengl) { This check should go in features.prf > Source/WebCore/WebCore.pri:213 > + CONFIG += graphics3d > + DEFINES += WTF_USE_3D_GRAPHICS=1 And instead of going through an intermediate CONFIG variable, define only DEFINES += WTF_USE_3D_GRAPHICS=1, and check for that wherever you have contains(CONFIG, graphics3d). I know, it's a bit cumbersome, but it's the pattern we use everywhere else for features. > Source/WebCore/WebCore.pri:214 > + contains(QT_CONFIG, opengles2): LIBS += -lEGL This part will then still live inside contains(DEFINES, WTF_USE_3D_GRAPHICS=1) here.
Created attachment 150852 [details] Patch
(In reply to comment #4) > (From update of attachment 150849 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150849&action=review > instead of going through an intermediate CONFIG variable, define only DEFINES += WTF_USE_3D_GRAPHICS=1, and check for that wherever you have contains(CONFIG, graphics3d). I know, it's a bit cumbersome, but it's the pattern we use everywhere else for features. > > > Source/WebCore/WebCore.pri:214 > > + contains(QT_CONFIG, opengles2): LIBS += -lEGL > > This part will then still live inside contains(DEFINES, WTF_USE_3D_GRAPHICS=1) here. Got it :) All of that should be fixed in the new patch.
Created attachment 150856 [details] Patch
Comment on attachment 150856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150856&action=review looks good from a Qt pov > Source/WebCore/Target.pri:3943 > + Extra line > Source/WebCore/Target.pri:4101 > + Whitespace-hunk
Created attachment 151006 [details] Patch
Comment on attachment 151006 [details] Patch This looks good to me, but since you are turning on WTF_USE_3D_GRAPHICS for all ports, perhaps it would be good to update all ENABLE(WEBGL)to read USE(3D_GRAPHICS) in all GC3D files? For instance, skia/GraphicsContext3DSkia.cpp is unchanged.
Created attachment 151159 [details] Patch
(In reply to comment #10) > (From update of attachment 151006 [details]) > This looks good to me, but since you are turning on WTF_USE_3D_GRAPHICS for all ports, perhaps it would be good to update all ENABLE(WEBGL)to read USE(3D_GRAPHICS) in all GC3D files? For instance, skia/GraphicsContext3DSkia.cpp is unchanged. OK, I uploaded another patch with the proposed approach. Not sure which one is better; I agree that all ports should move to using USE(3D_GRAPHICS) for GC3D, DrawingBuffer and the CSS shaders stuff, but maybe it's better to do that gradually whenever the port sees fit rather than try to cram it all in this patch? Either way, both patches are up to review and I'm OK with either way.
Created attachment 151263 [details] Patch for landing
Comment on attachment 151263 [details] Patch for landing Rejecting attachment 151263 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ext3DSkia.cpp patching file Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.h patching file Source/WebCore/platform/qt/QWebPageClient.h patching file Source/WebKit/qt/WebCoreSupport/PageClientQt.cpp patching file Source/WebKit/qt/WebCoreSupport/PageClientQt.h patching file Tools/ChangeLog patching file Tools/qmake/mkspecs/features/features.prf Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/13174142
Created attachment 151303 [details] Patch for landing
Comment on attachment 151303 [details] Patch for landing Rejecting attachment 151303 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 11988 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 47>At revision 11988. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/13176035
Created attachment 151338 [details] Patch for landing
Comment on attachment 151338 [details] Patch for landing Rejecting attachment 151338 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: e/platform/graphics/texmap/GraphicsLayerTextureMapper.h patching file Source/WebCore/platform/qt/QWebPageClient.h patching file Source/WebKit/qt/ChangeLog patching file Source/WebKit/qt/WebCoreSupport/PageClientQt.cpp patching file Source/WebKit/qt/WebCoreSupport/PageClientQt.h patching file Tools/ChangeLog patching file Tools/qmake/mkspecs/features/features.prf Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/13161456
Created attachment 151347 [details] Patch for landing
Created attachment 151350 [details] Patch for landing
Comment on attachment 151350 [details] Patch for landing Rejecting attachment 151350 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ext3DSkia.cpp patching file Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.h patching file Source/WebCore/platform/qt/QWebPageClient.h patching file Source/WebKit/qt/WebCoreSupport/PageClientQt.cpp patching file Source/WebKit/qt/WebCoreSupport/PageClientQt.h patching file Tools/ChangeLog patching file Tools/qmake/mkspecs/features/features.prf Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/13161482
Committed manually, http://trac.webkit.org/changeset/122175
Sorry for the faulty commit message, an oversight.
Re-opened since this is blocked by 90847
(In reply to comment #22) > Committed manually, http://trac.webkit.org/changeset/122175 It broke the Qt win build: https://bugs.webkit.org/show_bug.cgi?id=90850
(In reply to comment #22) > Committed manually, http://trac.webkit.org/changeset/122175 GTK buidlfix: https://trac.webkit.org/changeset/122202 Chromium buildfix: https://trac.webkit.org/changeset/122193