Bug 220896

<rdar://problem/73539682>

This turned out to be a larger task than originally expected and it's still underway. https://bugs.chromium.org/p/angleproject/issues/detail?id=5505 is the tracking ANGLE bug.

When this is done, it's likely we'll need to overwrite Apple's version of ANGLE with the upstreamed one rather than attempting to rebase all of the changes forward, because there will be many collisions that have already been resolved upstream.

The change to thread-local storage from Bug 228240 needs to be carefully preserved during this process. More care may be necessary in other areas as well.

Perhaps this should be blocked on Bug 232064 to indicate that we should ideally revise the update-angle script to handle this scenario.

Created attachment 443636 [details]
changes.diff from experimental rebase

Comment on attachment 443636 [details]
changes.diff from experimental rebase

View in context: https://bugs.webkit.org/attachment.cgi?id=443636&action=review

Publishing first comments draft (Up to State.cpp)

> include/platform/FeaturesMtl.h:30
> +    Feature hasExplicitMemBarrier = {"has_explicit_mem_barrier", FeatureCategory::MetalFeatures,

Can Drop and unify this change.

> include/platform/FeaturesMtl.h:43
> +    Feature hasStencilOutput = {"has_stencil_output", FeatureCategory::MetalFeatures,

Can drop and unify this change.

> include/platform/PlatformMethods.h:19
> +#    if defined(__GNUC__) || defined(__clang__)

Drop and unify

> src/common/angleutils.h:-224
> -size_t FormatStringIntoVector(const char *fmt, va_list vararg, std::vector<char> &buffer);

In source angle repo, Keep

> src/common/utilities.cpp:9
> +// Older clang versions have a false positive on this warning here.

Possibly necessary?

> src/common/utilities.cpp:488
> +/**

Drop, rewritten in Upstream

> src/common/utilities.h:40
> +int VariableAttributeCount(GLenum type);

Drop, unneeded

> src/compiler/preprocessor/preprocessor_tab_autogen.cpp:3
> +/* Apple Note: For the avoidance of doubt, Apple elects to distribute this file under the terms of the BSD license. */

Very much keep

> src/compiler/translator/BaseTypes.h:1507
> +    case EvqSampleMask:             return "SampleMask";

Drop, doubled up below.

> src/compiler/translator/CodeGen.cpp:26
> +#ifdef ANGLE_ENABLE_METAL_SPIRV

Keep, needed for compiling without SpirV

> src/compiler/translator/Common.h:217
> +    constexpr const char *formatStr = std::numeric_limits<T>::is_signed ? "%d" : "%u";

Possibly back port?

> src/compiler/translator/IntermNode.cpp:438
> +TIntermBlock::TIntermBlock(std::initializer_list<TIntermNode *> stmts)

Drop, not needed in current transpiler

> src/compiler/translator/IntermNode.cpp:539
> +    auto &seq = *getSequence();

Return to upstream.

> src/compiler/translator/IntermNode.cpp:550
> +    auto &seq = *getSequence();

Return to upstream

> src/compiler/translator/IntermNode.cpp:-556
> -    if (position > getSequence()->size())

Return to upstream, just refactoring.

> src/compiler/translator/TranslatorMetal.cpp:46
> +constexpr ImmutableString kDiscardWrapperFuncName  = ImmutableString("ANGLEDiscardWrapper");

Return to upstream, unused translator

> src/compiler/translator/TranslatorMetal.cpp:77
> +ANGLE_NO_DISCARD bool EmulateInstanceID(TCompiler *compiler,

All hardware supports instancing, drop.

> src/compiler/translator/TranslatorMetal.cpp:168
> +        if (mEmulatedInstanceID)

All hardware supports instancing

> src/compiler/translator/TranslatorMetal.cpp:-288
> -    sink << "layout (constant_id=0) const bool " << mtl::kRasterizerDiscardEnabledConstName;

Use namespace

> src/compiler/translator/TranslatorMetal.cpp:367
> +// If the MSL fragment shader is non-void, we need to ensure

Probably drop, no longer in main repo

> src/compiler/translator/TranslatorMetal.h:47
> +    void enableEmulatedInstanceID(bool e) { mEmulatedInstanceID = e; }

Drop instanced ID

> src/compiler/translator/TranslatorMetalDirect/ConstantNames.h:1
> +//

Refactor: Either back port or drop.

> src/compiler/translator/TranslatorMetalDirect/Debug.h:1
> +//

Remove

> src/compiler/translator/TranslatorMetalDirect/EnvironmentVariable.h:1
> +//

Remove

> src/compiler/translator/TranslatorMetalUtils.cpp:1
> +//

Keep

> src/compiler/translator/TranslatorMetalUtils.h:1
> +//

Keep

> src/compiler/translator/ValidateAST.cpp:-167
> -            if (mParent.find(child) != mParent.end())

Revert

> src/compiler/translator/glslang_tab_autogen.cpp:3
> +/* Apple Note: For the avoidance of doubt, Apple elects to distribute this file under the terms of the BSD license. */

Keep

> src/compiler/translator/glslang_tab_autogen.h:3
> +/* Apple Note: For the avoidance of doubt, Apple elects to distribute this file under the terms of the BSD license. */

keep

> src/compiler/translator/tree_ops/PruneEmptyCases.cpp:20
> +bool AreEmptyBlocks(const TIntermSequence *statements);

Can drop the const to unify

> src/compiler/translator/tree_ops/d3d/RemoveSwitchFallThrough.cpp:47
> +    void outputSequence(const TIntermSequence *sequence, size_t startIndex);

Drop const to unify

> src/compiler/translator/util.cpp:800
> +bool IsOutputMetalDirect(ShShaderOutput output)

Drop

> src/gpu_info_util/SystemInfo_apple.mm:-2
> -// Copyright 2020 The ANGLE Project Authors. All rights reserved.

Use ANGLE Project Authors

> src/gpu_info_util/SystemInfo_macos.mm:362
> +#if defined(ANGLE_PLATFORM_MACCATALYST) && defined(ANGLE_CPU_ARM64)

Keep, from upstream

> src/libANGLE/Context.cpp:9675
> +

Just whitespace

> src/libANGLE/Display.cpp:1449
> +Error Display::makeCurrent(const Thread *thread,

Drop, replaced by new function

> src/libANGLE/Display.h:182
> +    Error makeCurrent(const Thread *thread,

Drop, replaced by new function

> src/libANGLE/Program.h:297
> +    void setLinkedTransformFeedbackVaryings(

Drop, in Program.h now.

> src/libANGLE/ProgramExecutable.h:225
> +    void setLinkedTransformFeedbackVaryings(

Remove, in Program.h now.

> src/libANGLE/State.cpp:9
> +// Older clang versions have a false positive on this warning here.

From previous changes.diff. New clang may be ok now?

Created attachment 445467 [details]
Patch

Created attachment 445495 [details]
Patch

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

Created attachment 445635 [details]
Patch

Created attachment 445652 [details]
Fix WinCairo build

Please merge this fix for your next patch.

Created attachment 445812 [details]
Patch

Created attachment 445817 [details]
Patch

Created attachment 445892 [details]
Patch

Created attachment 446103 [details]
Patch

Created attachment 446107 [details]
Patch

The code review tool doesn't work for a patch this large.

I've applied an earlier iteration of this patch locally and examined the changes.diff from upstream; it looks good. The vast majority of WebKit's changes have been upstreamed, and over a year's worth of divergent development has been resolved.

Great work Kyle getting this to this point. Please consider this r+ by me.

The WinCairo failure here is an infrastructure problem where a new file wasn't properly cleaned by the bot.

Committed revision 286603.

https://trac.webkit.org/changeset/286603/webkit

Fantastic work Kyle managing this difficult forward roll! All subsequent ones will be much easier, and looking forward to collaborating on them!

Committed r286638 (244951@trunk): <https://commits.webkit.org/244951@trunk>

*** Bug 233817 has been marked as a duplicate of this bug. ***

*** Bug 232905 has been marked as a duplicate of this bug. ***

*** Bug 230216 has been marked as a duplicate of this bug. ***

Turns out it was pretty risky to diverge like this and then roll in so many changes. It seems we don’t have enough regression test coverage. Since the roll, WebKit testing has uncovered these three regressions:

Bug 236427
Bug 236733
Bug 236699