WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
222685
Metal-ANGLE: Shared memory texture tests failing in iOS Simulator
https://bugs.webkit.org/show_bug.cgi?id=222685
Summary
Metal-ANGLE: Shared memory texture tests failing in iOS Simulator
Dean Jackson
Reported
2021-03-03 13:47:18 PST
The following tests fail on the iOS Simulator with the ANGLE Metal backend. webgl/1.0.3/conformance/textures/tex-image-and-sub-image-2d-with-array-buffer-view.html [ Failure ] webgl/1.0.3/conformance/textures/texture-mips.html [ Failure ] According to Kyle, the issue is the resource type of the textures (which can't be shared on Intel macOS), and the simulator code not expecting that.
Attachments
Patch
(8.42 KB, patch)
2021-04-16 17:33 PDT
,
Kyle Piddington
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(8.50 KB, patch)
2021-04-16 17:58 PDT
,
Dean Jackson
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(4.64 KB, patch)
2021-04-27 15:39 PDT
,
Kyle Piddington
no flags
Details
Formatted Diff
Diff
Patch
(8.64 KB, patch)
2021-04-27 18:05 PDT
,
Kyle Piddington
no flags
Details
Formatted Diff
Diff
Patch
(10.91 KB, patch)
2021-04-28 11:18 PDT
,
Kyle Piddington
no flags
Details
Formatted Diff
Diff
Patch
(10.30 KB, patch)
2021-04-28 11:19 PDT
,
Kyle Piddington
no flags
Details
Formatted Diff
Diff
Patch
(10.34 KB, patch)
2021-04-29 11:52 PDT
,
Kyle Piddington
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(10.19 KB, patch)
2021-04-29 12:11 PDT
,
Kyle Piddington
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(10.19 KB, patch)
2021-04-29 12:33 PDT
,
Kyle Piddington
no flags
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2021-03-03 13:47:27 PST
<
rdar://problem/75001722
>
Kyle Piddington
Comment 2
2021-04-16 17:33:56 PDT
Created
attachment 426297
[details]
Patch
EWS Watchlist
Comment 3
2021-04-16 17:34:44 PDT
Note that there are important steps to take when updating ANGLE. See
https://trac.webkit.org/wiki/UpdatingANGLE
Dean Jackson
Comment 4
2021-04-16 17:58:26 PDT
Created
attachment 426301
[details]
Patch
EWS
Comment 5
2021-04-16 19:06:22 PDT
Committed
r276190
(
236672@main
): <
https://commits.webkit.org/236672@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 426301
[details]
.
Truitt Savell
Comment 6
2021-04-23 11:16:39 PDT
Reverted
r276190
for reason: broke a test internally. Committed
r276507
(
236964@main
): <
https://commits.webkit.org/236964@main
>
Kyle Piddington
Comment 7
2021-04-27 15:39:17 PDT
Created
attachment 427205
[details]
Patch
Kyle Piddington
Comment 8
2021-04-27 18:05:40 PDT
Created
attachment 427224
[details]
Patch
Kyle Piddington
Comment 9
2021-04-28 11:18:10 PDT
Created
attachment 427281
[details]
Patch
Kyle Piddington
Comment 10
2021-04-28 11:19:08 PDT
Created
attachment 427282
[details]
Patch
Kenneth Russell
Comment 11
2021-04-28 19:14:54 PDT
Comment on
attachment 427282
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=427282&action=review
Looks good to me. Couple of small comments and suggestions. It looks like the win EWS failure is an unrelated flake. r+
> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/TextureMtl.mm:308 > + size_t stagingBufferSize = stagingBuffer2DImageSize * regionSize.depth;
Are there any padding constraints for certain texture formats, in particular on the row pitch?
> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/TextureMtl.mm:337 > + size_t stagingBuffer2DImageSize = bytesPer2DImage;
This helper function is identical to the one above except for the computation of stagingBuffer2DImageSize. Would it be worth coalescing them into one with a "bool isCompressedFormat" argument or similar?
> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/TextureMtl.mm:490 > + //Block-compressed formats need a bit of massaging for copy.
Space after // (clang-format will fix this during upstreaming)
> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/TextureMtl.mm:496 > + //ASSERT(region.size.height % fmt.compressedBlockHeight == 0);
Remove commented-out code.
> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/TextureMtl.mm:535 > + ANGLE_TRY(UploadTextureContentsWithStagingBuffer(contextMtl,textureAngleFormat,region,mipmapLevel,slice,data,bytesPerRow,bytesPer2DImage,texture));
Separate arguments with a space (clang-format will fix this for you during upstreaming, anyway).
> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_utils.mm:156 > + index.getType() != gl::TextureType::_2DMultisampleArray && forceGPUInitialization == false)
&& !forceGPUInitialization
Kyle Piddington
Comment 12
2021-04-29 11:52:49 PDT
Created
attachment 427363
[details]
Patch
Kyle Piddington
Comment 13
2021-04-29 11:53:31 PDT
Comment on
attachment 426301
[details]
Patch This one should be dropped.
Kyle Piddington
Comment 14
2021-04-29 12:11:24 PDT
Created
attachment 427366
[details]
Patch
Kyle Piddington
Comment 15
2021-04-29 12:33:20 PDT
Created
attachment 427369
[details]
Patch
Kenneth Russell
Comment 16
2021-04-29 12:46:44 PDT
Comment on
attachment 427369
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=427369&action=review
Still looks good; two small suggestions.
> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/TextureMtl.mm:325 > +angle::Result CopyCompressedTextureContentsToStagingBuffer(
Same comment as before about considering unifying these entry points with a bool argument.
> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/TextureMtl.mm:502 > + //Copy to staging buffer before uploading to texture.
Space after "//"; also suggest putting these comments inside the if/else blocks to make them easier to read.
EWS
Comment 17
2021-04-29 16:57:35 PDT
kpiddington@apple.com
does not have committer permissions according to
https://raw.githubusercontent.com/WebKit/WebKit/main/Tools/Scripts/webkitpy/common/config/contributors.json
. Rejecting
attachment 427369
[details]
from commit queue.
Kenneth Russell
Comment 18
2021-04-29 17:16:36 PDT
Comment on
attachment 427369
[details]
Patch CQ'ing for you Kyle.
EWS
Comment 19
2021-04-29 17:22:22 PDT
Committed
r276825
(
237180@main
): <
https://commits.webkit.org/237180@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 427369
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug