Bug 201156 - Update ANGLE
Summary: Update ANGLE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: ANGLE (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Darpinian
URL:
Keywords: InRadar
Depends on: 201191
Blocks: webglangle 201785 235284
  Show dependency treegraph
 
Reported: 2019-08-26 15:34 PDT by James Darpinian
Modified: 2022-01-16 14:33 PST (History)
22 users (show)

See Also:


Attachments
Patch (20.73 MB, patch)
2019-08-26 15:55 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
just obsoleting this patch manually because webkit-patch hangs when it tries (1 bytes, patch)
2019-08-26 17:10 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
Patch (20.73 MB, patch)
2019-08-26 17:13 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
Reuploading patch after svn-apply fix (20.73 MB, patch)
2019-08-27 22:57 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
patch (20.29 MB, patch)
2019-08-28 14:24 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
patch that compiles but doesn't pass tests (20.29 MB, patch)
2019-08-28 17:07 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (22.62 MB, patch)
2019-09-11 17:02 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
Warning fix for older clang (22.62 MB, patch)
2019-09-11 18:24 PDT, James Darpinian
achristensen: review+
Details | Formatted Diff | Diff
Fix wincairo and Linux GTK debug builds (22.62 MB, patch)
2019-09-12 15:29 PDT, James Darpinian
no flags Details | Formatted Diff | Diff
Remove commit.h copying build steps (22.64 MB, patch)
2019-09-12 17:14 PDT, James Darpinian
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Darpinian 2019-08-26 15:34:08 PDT
Update ANGLE
Comment 1 James Darpinian 2019-08-26 15:55:42 PDT
Created attachment 377279 [details]
Patch
Comment 2 James Darpinian 2019-08-26 17:10:15 PDT
Created attachment 377293 [details]
just obsoleting this patch manually because webkit-patch hangs when it tries
Comment 3 James Darpinian 2019-08-26 17:13:21 PDT
Created attachment 377294 [details]
Patch
Comment 4 James Darpinian 2019-08-26 17:35:05 PDT
Seems like svn-apply can't handle git patches that only change the executable bit of a file.
Comment 5 James Darpinian 2019-08-27 11:22:30 PDT
Filed bug 201191 to fix svn-apply script to stop crashing on this patch.
Comment 6 James Darpinian 2019-08-27 22:57:35 PDT
Created attachment 377427 [details]
Reuploading patch after svn-apply fix
Comment 7 James Darpinian 2019-08-28 11:44:08 PDT
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.
Comment 8 Alex Christensen 2019-08-28 11:44:59 PDT
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.
Comment 9 Alex Christensen 2019-08-28 13:25:19 PDT
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.
Comment 10 James Darpinian 2019-08-28 14:23:48 PDT
Great! I will wait for that.
Comment 11 Alex Christensen 2019-08-28 14:24:53 PDT
Created attachment 377488 [details]
patch
Comment 12 Alex Christensen 2019-08-28 16:26:05 PDT
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?
Comment 13 Alex Christensen 2019-08-28 17:05:59 PDT
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.
Comment 14 Alex Christensen 2019-08-28 17:07:24 PDT
Created attachment 377513 [details]
patch that compiles but doesn't pass tests
Comment 15 James Darpinian 2019-08-28 17:28:00 PDT
Sure, I'll take a look tomorrow.
Comment 16 James Darpinian 2019-09-11 17:02:20 PDT
Created attachment 378601 [details]
Patch
Comment 17 James Darpinian 2019-09-11 17:04:00 PDT
Sorry for the delay. Try my latest patch. fast/canvas/webgl/error-reporting.html is passing for me with this patch.
Comment 18 EWS Watchlist 2019-09-11 17:11:17 PDT
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Comment 19 Alex Christensen 2019-09-11 17:12:53 PDT
What did you change?
Comment 20 James Darpinian 2019-09-11 17:29:21 PDT
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.
Comment 21 James Darpinian 2019-09-11 18:24:25 PDT
Created attachment 378610 [details]
Warning fix for older clang
Comment 22 Alex Christensen 2019-09-11 20:08:21 PDT
http://trac.webkit.org/r249791
Comment 23 Radar WebKit Bug Importer 2019-09-11 20:10:03 PDT
<rdar://problem/55288132>
Comment 24 Radar WebKit Bug Importer 2019-09-11 20:10:15 PDT
<rdar://problem/55288134>
Comment 25 Ryan Haddad 2019-09-11 22:06:57 PDT
Reverted r249791 for reason:

Breaks internal production builds.

Committed r249793: <https://trac.webkit.org/changeset/249793>
Comment 26 Ryan Haddad 2019-09-11 22:08:50 PDT
(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.
Comment 27 Fujii Hironori 2019-09-11 22:36:08 PDT
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)
Comment 28 James Darpinian 2019-09-12 15:29:18 PDT
Created attachment 378683 [details]
Fix wincairo and Linux GTK debug builds
Comment 29 Alex Christensen 2019-09-12 15:41:10 PDT
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.
Comment 30 James Darpinian 2019-09-12 15:48:38 PDT
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.
Comment 31 Alex Christensen 2019-09-12 15:49:57 PDT
That was an internal build failure from an important internal build system.  The committed code needs to build without such a step.
Comment 32 Alex Christensen 2019-09-12 16:55:41 PDT
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.
Comment 33 Alex Christensen 2019-09-12 17:02:35 PDT
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
Comment 34 James Darpinian 2019-09-12 17:06:12 PDT
I'll have a patch soon that copies the commit.h file to the right place rather than changing the source code.
Comment 35 Alex Christensen 2019-09-12 17:07:16 PDT
Don't bother.  I'll commit this after verifying it works, then we can do small diffs from it for things like that.
Comment 36 James Darpinian 2019-09-12 17:10:12 PDT
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.
Comment 37 James Darpinian 2019-09-12 17:14:27 PDT
Created attachment 378691 [details]
Remove commit.h copying build steps
Comment 38 Alex Christensen 2019-09-12 18:01:45 PDT
http://trac.webkit.org/r249823
Comment 39 Aakash Jain 2019-09-12 22:29:42 PDT
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)
Comment 40 James Darpinian 2019-09-13 16:42:16 PDT
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.
Comment 41 Russell Epstein 2019-09-13 17:13:58 PDT
Tracking test failure in https://bugs.webkit.org/show_bug.cgi?id=201784

Marked test as failing in r249860.