Created attachment 377279 [details] Patch

Created attachment 377293 [details] just obsoleting this patch manually because webkit-patch hangs when it tries

Created attachment 377294 [details] Patch

Seems like svn-apply can't handle git patches that only change the executable bit of a file.

Filed bug 201191 to fix svn-apply script to stop crashing on this patch.

Created attachment 377427 [details] Reuploading patch after svn-apply fix

The compile error on mac seems to be specific to the 9.4.1 version of XCode. It compiles without errors on XCode 10.3. I am able to reproduce locally after installing 9.4.1, investigating now.

Applying and building, I get this: In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: In file included from src/libANGLE/trace.h:13: src/third_party/trace_event/trace_event.h:663:49: error: too few arguments to function call, expected 10, have 9 flags); ^ In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: In file included from src/libANGLE/trace.h:13: In file included from src/third_party/trace_event/trace_event.h:147: src/common/event_tracer.h:14:1: note: 'AddTraceEvent' declared here angle::TraceEventHandle AddTraceEvent(PlatformMethods *platform, ^ In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: In file included from src/libANGLE/trace.h:13: src/third_party/trace_event/trace_event.h:680:70: error: too few arguments to function call, expected 10, have 9 argTypes, argValues, flags); ^ In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: In file included from src/libANGLE/trace.h:13: In file included from src/third_party/trace_event/trace_event.h:147: src/common/event_tracer.h:14:1: note: 'AddTraceEvent' declared here angle::TraceEventHandle AddTraceEvent(PlatformMethods *platform, ^ In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: In file included from src/libANGLE/trace.h:13: src/third_party/trace_event/trace_event.h:701:70: error: too few arguments to function call, expected 10, have 9 argTypes, argValues, flags); ^ In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: In file included from src/libANGLE/trace.h:13: In file included from src/third_party/trace_event/trace_event.h:147: src/common/event_tracer.h:14:1: note: 'AddTraceEvent' declared here angle::TraceEventHandle AddTraceEvent(PlatformMethods *platform, ^ In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: In file included from src/libANGLE/trace.h:13: src/third_party/trace_event/trace_event.h:732:66: error: too few arguments to function call, expected 10, have 9 TRACE_EVENT_FLAG_NONE); ^ In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: In file included from src/libANGLE/trace.h:13: In file included from src/third_party/trace_event/trace_event.h:147: src/common/event_tracer.h:14:1: note: 'AddTraceEvent' declared here angle::TraceEventHandle AddTraceEvent(PlatformMethods *platform, ^ /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:553:37: error: too many arguments provided to function-like macro invocation ANGLE_TRACE_EVENT0("gpu.angle", "egl::Display::initialize"); ^ In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: In file included from src/libANGLE/trace.h:13: src/third_party/trace_event/trace_event.h:158:9: note: macro 'TRACE_EVENT0' defined here #define TRACE_EVENT0(category, name) INTERNAL_TRACE_EVENT_ADD_SCOPED(category, name) ^ /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:553:5: error: use of undeclared identifier 'TRACE_EVENT0' ANGLE_TRACE_EVENT0("gpu.angle", "egl::Display::initialize"); ^ In file included from /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/libANGLE/Display.cpp:42: src/libANGLE/trace.h:22:45: note: expanded from macro 'ANGLE_TRACE_EVENT0' #define ANGLE_TRACE_EVENT0(CATEGORY, EVENT) TRACE_EVENT0(ANGLEPlatformCurrent(), CATEGORY, EVENT) ^ 6 errors generated.

I've got an ANGLE update to that same revision that works for me locally. I'm going to double check tests, iOS build, and a few other things then commit it.

Great! I will wait for that.

Created attachment 377488 [details] patch

When I make this update, ANGLEWebKitBridge::compileShaderSource is returning false because sh::Compile is returning false and sh::GetInfoLog is returning these strings for vertex and fragment shaders, respectively: ERROR: 0:11: 'gl_Position' : undeclared identifier ERROR: 0:8: 'gl_FragColor' : undeclared identifier Has something changed in the way we need to tell ANGLE that this is a vertex shader and fragment shader?

When running the LayoutTest fast/canvas/webgl/error-reporting.html with this patch applied, symbol is null in TParseContext::parseVariableIdentifier when it's parsing "gl_Position". Could you take a look, James? Here's my stack: ... #5 0x11c248d2b in sh::TParseContext::parseVariableIdentifier(sh::TSourceLoc const&, sh::ImmutableString const&, sh::TSymbol const*) at /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/compiler/translator/ParseContext.cpp:1887 #6 0x11c1de03d in yyparse(sh::TParseContext*, void*) at /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/compiler/translator/glslang_tab.cpp:2524 #7 0x11c1e4e9c in glslang_parse(sh::TParseContext*) at /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/compiler/translator/glslang_tab.cpp:5208 #8 0x11c25c793 in sh::PaParseStrings(unsigned long, char const* const*, int const*, sh::TParseContext*) at /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/compiler/translator/ParseContext.cpp:6095 #9 0x11c1a2eb0 in sh::TCompiler::compileTreeImpl(char const* const*, unsigned long, unsigned long long) at /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/compiler/translator/Compiler.cpp:397 #10 0x11c1a69d0 in sh::TCompiler::compile(char const* const*, unsigned long, unsigned long long) at /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/compiler/translator/Compiler.cpp:925 #11 0x11c28657c in sh::Compile(void*, char const* const*, unsigned long, unsigned long long) at /Users/alexchristensen/code/OpenSource/Source/ThirdParty/ANGLE/src/compiler/translator/ShaderLang.cpp:358 #12 0x11b3030d2 in WebCore::ANGLEWebKitBridge::compileShaderSource(char const*, WebCore::ANGLEShaderType, WTF::String&, WTF::String&, WTF::Vector<std::__1::pair<WebCore::ANGLEShaderSymbolType, sh::ShaderVariable>, 0ul, WTF::CrashOnOverflow, 16ul>&, unsigned long long) at /Users/alexchristensen/code/OpenSource/Source/WebCore/platform/graphics/ANGLEWebKitBridge.cpp:186 #13 0x11b52d0ee in WebCore::Extensions3DOpenGLCommon::getTranslatedShaderSourceANGLE(unsigned int) at /Users/alexchristensen/code/OpenSource/Source/WebCore/platform/graphics/opengl/Extensions3DOpenGLCommon.cpp:199 #14 0x11b53275e in WebCore::GraphicsContext3D::compileShader(unsigned int) at /Users/alexchristensen/code/OpenSource/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:663 ... It feels like something new is required when instantiating and using a Compiler.

Created attachment 377513 [details] patch that compiles but doesn't pass tests

Sure, I'll take a look tomorrow.

Created attachment 378601 [details] Patch

Sorry for the delay. Try my latest patch. fast/canvas/webgl/error-reporting.html is passing for me with this patch.

Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE

What did you change?

Since my last patch I've updated ANGLE upstream with the goal of allowing WebKit to update ANGLE by directly copying it into the repo without code changes. I am not sure why your patch was failing that test. Perhaps some older files didn't get deleted or newer files didn't get added to the xcocdeproj file, or both.

Created attachment 378610 [details] Warning fix for older clang

<rdar://problem/55288132>

<rdar://problem/55288134>

Reverted r249791 for reason: Breaks internal production builds. Committed r249793: <https://trac.webkit.org/changeset/249793>

(In reply to Ryan Haddad from comment #25) > Reverted r249791 for reason: > > Breaks internal production builds. > > Committed r249793: <https://trac.webkit.org/changeset/249793> I emailed Alex an example of the failure and will work with him to get it resolved before re-landing this change.

WinCairo builds got broken. https://build.webkit.org/builders/WinCairo%2064-bit%20WKL%20Release%20%28Build%29/builds/11895 > ANGLE.lib(Context.cpp.obj) : error LNK2019: unresolved external symbol "public: __cdecl angle::FrameCapture::FrameCapture(void)" (??0FrameCapture@angle@@QEAA@XZ) referenced in function "public: __cdecl gl::Context::Context(class egl::Display *,struct egl::Config const *,class gl::Context const *,class gl::TextureManager *,class gl::MemoryProgramCache *,unsigned int,class egl::AttributeMap const &,struct egl::DisplayExtensions const &,struct egl::ClientExtensions const &)" (??0Context@gl@@QEAA@PEAVDisplay@egl@@PEBUConfig@3@PEBV01@PEAVTextureManager@1@PEAVMemoryProgramCache@1@IAEBVAttributeMap@3@AEBUDisplayExtensions@3@AEBUClientExtensions@3@@Z) > ANGLE.lib(Context.cpp.obj) : error LNK2019: unresolved external symbol "public: __cdecl angle::FrameCapture::~FrameCapture(void)" (??1FrameCapture@angle@@QEAA@XZ) referenced in function "public: __cdecl std::unique_ptr<class angle::FrameCapture,struct std::default_delete<class angle::FrameCapture> >::~unique_ptr<class angle::FrameCapture,struct std::default_delete<class angle::FrameCapture> >(void)" (??1?$unique_ptr@VFrameCapture@angle@@U?$default_delete@VFrameCapture@angle@@@std@@@std@@QEAA@XZ) > ANGLE.lib(Context.cpp.obj) : error LNK2019: unresolved external symbol "public: void __cdecl angle::FrameCapture::onEndFrame(class gl::Context const *)" (?onEndFrame@FrameCapture@angle@@QEAAXPEBVContext@gl@@@Z) referenced in function "public: void __cdecl gl::Context::onPostSwap(void)const " (?onPostSwap@Context@gl@@QEBAXXZ)

Created attachment 378683 [details] Fix wincairo and Linux GTK debug builds

There was also a build failure when building src/libANGLE/BlobCache.cpp:12: src/common/version.h:10:10: fatal error: 'id/commit.h' file not found #include "id/commit.h" ^~~~~~~~~~~~~ 1 error generated.

Is that the internal build failure? I added a build step to the xcodeproj file and CMakeLists.txt to copy that header. If you use a different build system internally, then you can add that build step there as well. Or if you prefer, I can change the update-angle.sh script to copy the header as part of the import process.

That was an internal build failure from an important internal build system. The committed code needs to build without such a step.

I verified that replacing "id/commit.h" with "commit.h" fixes that part of the internal build, but there's something else related to group writable files, sticky bits, and world writable files that it doesn't like.

Removing the new Copy Files phase fixed it: Index: Source/ThirdParty/ANGLE/ANGLE.xcodeproj/project.pbxproj =================================================================== --- Source/ThirdParty/ANGLE/ANGLE.xcodeproj/project.pbxproj (revision 249792) +++ Source/ThirdParty/ANGLE/ANGLE.xcodeproj/project.pbxproj (working copy) @@ -709,7 +709,6 @@ A3694FC623202C5200A83D8F /* BuiltinsWorkaroundGLSL.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A3694FC423202C5100A83D8F /* BuiltinsWorkaroundGLSL.cpp */; }; A3694FC723202C5200A83D8F /* BuiltinsWorkaroundGLSL.h in Headers */ = {isa = PBXBuildFile; fileRef = A3694FC523202C5200A83D8F /* BuiltinsWorkaroundGLSL.h */; }; A3E827A9230CAE2C00E76682 /* commit.h in Headers */ = {isa = PBXBuildFile; fileRef = A3E827A8230CAE2C00E76682 /* commit.h */; }; - A3E827AA230CAE3C00E76682 /* commit.h in CopyFiles */ = {isa = PBXBuildFile; fileRef = A3E827A8230CAE2C00E76682 /* commit.h */; }; FB39D76E120110FC00088E69 /* ShaderLang.h in Headers */ = {isa = PBXBuildFile; fileRef = FB39D2BF1200F3E600088E69 /* ShaderLang.h */; settings = {ATTRIBUTES = (Public, ); }; }; /* End PBXBuildFile section */ @@ -749,16 +748,6 @@ ); runOnlyForDeploymentPostprocessing = 1; }; - A3E827A7230CADE300E76682 /* CopyFiles */ = { - isa = PBXCopyFilesBuildPhase; - buildActionMask = 2147483647; - dstPath = "$(DERIVED_FILE_DIR)/include/id"; - dstSubfolderSpec = 0; - files = ( - A3E827AA230CAE3C00E76682 /* commit.h in CopyFiles */, - ); - runOnlyForDeploymentPostprocessing = 0; - }; /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ @@ -2901,7 +2890,6 @@ buildPhases = ( FB39D77B1201110C00088E69 /* Headers */, A3E827AB230CAF7700E76682 /* ShellScript */, - A3E827A7230CADE300E76682 /* CopyFiles */, FB39D0CE1200F0E300088E69 /* Sources */, FB39D0CF1200F0E300088E69 /* Frameworks */, 312BDB0B15FECAB00097EBC7 /* CopyFiles */, Index: Source/ThirdParty/ANGLE/src/common/version.h =================================================================== --- Source/ThirdParty/ANGLE/src/common/version.h (revision 249792) +++ Source/ThirdParty/ANGLE/src/common/version.h (working copy) @@ -7,7 +7,7 @@ #ifndef COMMON_VERSION_H_ #define COMMON_VERSION_H_ -#include "id/commit.h" +#include "commit.h" #define ANGLE_MAJOR_VERSION 2 #define ANGLE_MINOR_VERSION 1

I'll have a patch soon that copies the commit.h file to the right place rather than changing the source code.

Don't bother. I'll commit this after verifying it works, then we can do small diffs from it for things like that.

There are several other things to remove related to the commit.h copying. There's an include path that's no longer needed, plus changes to the CMakeLists.txt.

Created attachment 378691 [details] Remove commit.h copying build steps

It seems like this commit broke 'webgl/2.0.0/conformance/glsl/misc/shaders-with-invariance.html' test on iOS. (EWS also indicated that failure)

Looks like the shaders-with-invariance.html test is broken. It was updated upstream. WebKit's copy of the WebGL conformance tests should be updated.

Tracking test failure in https://bugs.webkit.org/show_bug.cgi?id=201784 Marked test as failing in r249860.