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-
Patch (8.50 KB, patch)
2021-04-16 17:58 PDT, Dean Jackson
ews-feeder: commit-queue-
Patch (4.64 KB, patch)
2021-04-27 15:39 PDT, Kyle Piddington
no flags
Patch (8.64 KB, patch)
2021-04-27 18:05 PDT, Kyle Piddington
no flags
Patch (10.91 KB, patch)
2021-04-28 11:18 PDT, Kyle Piddington
no flags
Patch (10.30 KB, patch)
2021-04-28 11:19 PDT, Kyle Piddington
no flags
Patch (10.34 KB, patch)
2021-04-29 11:52 PDT, Kyle Piddington
ews-feeder: commit-queue-
Patch (10.19 KB, patch)
2021-04-29 12:11 PDT, Kyle Piddington
ews-feeder: commit-queue-
Patch (10.19 KB, patch)
2021-04-29 12:33 PDT, Kyle Piddington
no flags
Radar WebKit Bug Importer
Comment 1 2021-03-03 13:47:27 PST
Kyle Piddington
Comment 2 2021-04-16 17:33:56 PDT
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
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
Kyle Piddington
Comment 8 2021-04-27 18:05:40 PDT
Kyle Piddington
Comment 9 2021-04-28 11:18:10 PDT
Kyle Piddington
Comment 10 2021-04-28 11:19:08 PDT
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
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
Kyle Piddington
Comment 15 2021-04-29 12:33:20 PDT
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
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.