Summary: | Metal-ANGLE: Shared memory texture tests failing in iOS Simulator | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||||||||||||||||||
Component: | WebGL | Assignee: | Dean Jackson <dino> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, dino, ews-watchlist, graouts, kbr, kondapallykalyan, kpiddington, tsavell, webkit-bug-importer | ||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
Attachments: |
|
Description
Dean Jackson
2021-03-03 13:47:18 PST
Created attachment 426297 [details]
Patch
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE Created attachment 426301 [details]
Patch
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]. Reverted r276190 for reason: broke a test internally. Committed r276507 (236964@main): <https://commits.webkit.org/236964@main> Created attachment 427205 [details]
Patch
Created attachment 427224 [details]
Patch
Created attachment 427281 [details]
Patch
Created attachment 427282 [details]
Patch
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 Created attachment 427363 [details]
Patch
Comment on attachment 426301 [details]
Patch
This one should be dropped.
Created attachment 427366 [details]
Patch
Created attachment 427369 [details]
Patch
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. 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. Comment on attachment 427369 [details]
Patch
CQ'ing for you Kyle.
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]. |