Currently, these two classes are doing basically the same thing when ANGLE is enabled. I think we should remove the ANGLE-specific code from NicosiaGCGLLayer and that NicosiaGCGLANGLELayer should be a sub-class of NicosiaImageBufferPipe so that they can share the code and OffscreenCanvas can benefit when we get copy-less buffer transfers for WebGL+ANGLE.
Created attachment 449058 [details] Patch
Perhaps it's coincidence, but this also seems to have fixed some flickering I was seeing prior to this patch - I don't believe this works in any significantly different way to before though, so maybe just a driver bug exposed by timing differences or something...
Comment on attachment 449058 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449058&action=review Great patch! Very helpful for the next refactorings. > Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.h:45 > -class GCGLANGLELayer; > +class GCGLANGLEPipe; Can we ifdef USE(ANGLE) like we do in the attribute definition? > Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLEPipe.h:51 > +class GCGLANGLEPipeSource; Leftover?
Thanks for the review! (In reply to Alejandro G. Castro from comment #3) > Comment on attachment 449058 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=449058&action=review > > Great patch! Very helpful for the next refactorings. > > > Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.h:45 > > -class GCGLANGLELayer; > > +class GCGLANGLEPipe; > > Can we ifdef USE(ANGLE) like we do in the attribute definition? I think it's convention to just declare the class regardless (at least in a previous patch where I did what you're suggesting, a review comment was to remove the #ifdef :)) > > Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLEPipe.h:51 > > +class GCGLANGLEPipeSource; > > Leftover? Nice catch, thanks :)
Created attachment 449159 [details] Patch
Committed r288014 (246040@main): <https://commits.webkit.org/246040@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449159 [details].
<rdar://problem/87596622>
This broke clean WPE builds, both clang and gcc. There's a bunch of errors, here's an extract: 1228/7908] Building CXX object Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEGLEnums_autogen.cpp.o FAILED: Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEGLEnums_autogen.cpp.o /usr/bin/sccache /usr/bin/c++ -DANGLE_ENABLE_ESSL -DANGLE_ENABLE_GLSL -DANGLE_PLATFORM_LINUX -DBUILDING_WITH_CMAKE=1 -DBUILDING_WPE__=1 -DBWRAP_EXECUTABLE=\"/usr/bin/bwrap\" -DDBUS_PROXY_EXECUTABLE=\"/usr/bin/xdg-dbus-proxy\" -DEGL_EGL_PROTOTYPES=0 -DGETTEXT_PACKAGE=\"WPE\" -DGL_GLES_PROTOTYPES=0 -DHAVE_CONFIG_H=1 -DJSC_GLIB_API_ENABLED -DLIBANGLE_IMPLEMENTATION -DPAS_BMALLOC=1 -I/app/webkit/Source/ThirdParty/ANGLE/include -I/app/webkit/Source/ThirdParty/ANGLE/include/KHR -I/app/webkit/Source/ThirdParty/ANGLE/src -I/app/webkit/Source/ThirdParty/ANGLE/src/common/third_party/base -I/app/webkit/Source/ThirdParty/ANGLE/third_party/zlib/google -I/app/webkit/WebKitBuild/Release/Source/ThirdParty/ANGLE/include -fdiagnostics-color=always -Wextra -Wall -Wno-expansion-to-defined -Wno-stringop-overread -Wno-nonnull -Wno-array-bounds -Wno-noexcept-type -Wno-psabi -Wno-misleading-indentation -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -Wno-tautological-compare -fno-strict-aliasing -fno-exceptions -fno-rtti -O3 -DNDEBUG -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Wno-cast-align -Wno-extra -Wno-suggest-attribute=format -Wno-undef -Wno-unused-parameter -Wno-return-type -Wno-comment -std=c++20 -MD -MT Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEGLEnums_autogen.cpp.o -MF Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEGLEnums_autogen.cpp.o.d -o Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEGLEnums_autogen.cpp.o -c /app/webkit/Source/ThirdParty/ANGLE/src/common/PackedEGLEnums_autogen.cpp [1230/7908] Building CXX object Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEnums.cpp.o FAILED: Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEnums.cpp.o /usr/bin/sccache /usr/bin/c++ -DANGLE_ENABLE_ESSL -DANGLE_ENABLE_GLSL -DANGLE_PLATFORM_LINUX -DBUILDING_WITH_CMAKE=1 -DBUILDING_WPE__=1 -DBWRAP_EXECUTABLE=\"/usr/bin/bwrap\" -DDBUS_PROXY_EXECUTABLE=\"/usr/bin/xdg-dbus-proxy\" -DEGL_EGL_PROTOTYPES=0 -DGETTEXT_PACKAGE=\"WPE\" -DGL_GLES_PROTOTYPES=0 -DHAVE_CONFIG_H=1 -DJSC_GLIB_API_ENABLED -DLIBANGLE_IMPLEMENTATION -DPAS_BMALLOC=1 -I/app/webkit/Source/ThirdParty/ANGLE/include -I/app/webkit/Source/ThirdParty/ANGLE/include/KHR -I/app/webkit/Source/ThirdParty/ANGLE/src -I/app/webkit/Source/ThirdParty/ANGLE/src/common/third_party/base -I/app/webkit/Source/ThirdParty/ANGLE/third_party/zlib/google -I/app/webkit/WebKitBuild/Release/Source/ThirdParty/ANGLE/include -fdiagnostics-color=always -Wextra -Wall -Wno-expansion-to-defined -Wno-stringop-overread -Wno-nonnull -Wno-array-bounds -Wno-noexcept-type -Wno-psabi -Wno-misleading-indentation -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -Wno-tautological-compare -fno-strict-aliasing -fno-exceptions -fno-rtti -O3 -DNDEBUG -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Wno-cast-align -Wno-extra -Wno-suggest-attribute=format -Wno-undef -Wno-unused-parameter -Wno-return-type -Wno-comment -std=c++20 -MD -MT Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEnums.cpp.o -MF Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEnums.cpp.o.d -o Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/common/PackedEnums.cpp.o -c /app/webkit/Source/ThirdParty/ANGLE/src/common/PackedEnums.cpp [1231/7908] Building CXX object Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/compiler/translator/tree_ops/gl/UseInterfaceBlockFields.cpp.o FAILED: Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/compiler/translator/tree_ops/gl/UseInterfaceBlockFields.cpp.o /usr/bin/sccache /usr/bin/c++ -DANGLE_ENABLE_ESSL -DANGLE_ENABLE_GLSL -DANGLE_PLATFORM_LINUX -DBUILDING_WITH_CMAKE=1 -DBUILDING_WPE__=1 -DBWRAP_EXECUTABLE=\"/usr/bin/bwrap\" -DDBUS_PROXY_EXECUTABLE=\"/usr/bin/xdg-dbus-proxy\" -DEGL_EGL_PROTOTYPES=0 -DGETTEXT_PACKAGE=\"WPE\" -DGL_GLES_PROTOTYPES=0 -DHAVE_CONFIG_H=1 -DJSC_GLIB_API_ENABLED -DLIBANGLE_IMPLEMENTATION -DPAS_BMALLOC=1 -I/app/webkit/Source/ThirdParty/ANGLE/include -I/app/webkit/Source/ThirdParty/ANGLE/include/KHR -I/app/webkit/Source/ThirdParty/ANGLE/src -I/app/webkit/Source/ThirdParty/ANGLE/src/common/third_party/base -I/app/webkit/Source/ThirdParty/ANGLE/third_party/zlib/google -I/app/webkit/WebKitBuild/Release/Source/ThirdParty/ANGLE/include -fdiagnostics-color=always -Wextra -Wall -Wno-expansion-to-defined -Wno-stringop-overread -Wno-nonnull -Wno-array-bounds -Wno-noexcept-type -Wno-psabi -Wno-misleading-indentation -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -Wno-tautological-compare -fno-strict-aliasing -fno-exceptions -fno-rtti -O3 -DNDEBUG -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Wno-cast-align -Wno-extra -Wno-suggest-attribute=format -Wno-undef -Wno-unused-parameter -Wno-return-type -Wno-comment -std=c++20 -MD -MT Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/compiler/translator/tree_ops/gl/UseInterfaceBlockFields.cpp.o -MF Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/compiler/translator/tree_ops/gl/UseInterfaceBlockFields.cpp.o.d -o Source/ThirdParty/ANGLE/CMakeFiles/ANGLE.dir/src/compiler/translator/tree_ops/gl/UseInterfaceBlockFields.cpp.o -c /app/webkit/Source/ThirdParty/ANGLE/src/compiler/translator/tree_ops/gl/UseInterfaceBlockFields.cpp
Ah sorry, this is happening elsewhere in my build, something related with an empty llint offsets headers...