[m134] metal: Specify storage binding usage for IOSurface textures explicitly Add a allowStorageBinding flag on SharedTextureMemoryIOSurfaceDescriptor which specifies whether TextureUsage::StorageBinding is allowed and if MTLTextureUsageShaderWrite is set on the IOSurface wrapping MTLTexture. The flag defaults to true to match current behavior, but we will change it to false once Chromium migrates to using the flag. Chromium will set the flag based on whether SHARED_IMAGE_USAGE_WEBGPU_STORAGE_TEXTURE usage is set on the IOSurfaceImageBacking. Bug: 388872587, 395975428 Change-Id: I8cd78711cf8600b77a3d14b58eaa6d026afe83d1 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/223216 Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> (cherry picked from commit dc74be4e7640c603c35f333d488f32c153f807c5) Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/226296 Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org> Reviewed-by: Loko Kung <lokokung@google.com> Commit-Queue: Loko Kung <lokokung@google.com>
diff --git a/src/dawn/dawn.json b/src/dawn/dawn.json index 30bb8ce..a5c191c 100644 --- a/src/dawn/dawn.json +++ b/src/dawn/dawn.json
@@ -1977,7 +1977,8 @@ "tags": ["dawn", "native"], "_comment": "TODO(crbug.com/dawn/1745): improve type safety", "members": [ - {"name": "io surface", "type": "void *"} + {"name": "io surface", "type": "void *"}, + {"name": "allow storage binding", "type": "bool", "default": "true"} ] }, "shared texture memory EGL image descriptor": {
diff --git a/src/dawn/native/SharedTextureMemory.h b/src/dawn/native/SharedTextureMemory.h index 1623e0c..c0f5316 100644 --- a/src/dawn/native/SharedTextureMemory.h +++ b/src/dawn/native/SharedTextureMemory.h
@@ -71,9 +71,10 @@ const SharedTextureMemoryDescriptor* descriptor, ObjectBase::ErrorTag tag); + MaybeError GetProperties(SharedTextureMemoryProperties* properties) const; + private: ResultOrError<Ref<TextureBase>> CreateTexture(const TextureDescriptor* rawDescriptor); - MaybeError GetProperties(SharedTextureMemoryProperties* properties) const; Ref<SharedResourceMemoryContents> CreateContents() override;
diff --git a/src/dawn/native/metal/SharedTextureMemoryMTL.h b/src/dawn/native/metal/SharedTextureMemoryMTL.h index 7cf5206..f487be8 100644 --- a/src/dawn/native/metal/SharedTextureMemoryMTL.h +++ b/src/dawn/native/metal/SharedTextureMemoryMTL.h
@@ -61,7 +61,9 @@ SharedTextureMemory(Device* device, StringView label, const SharedTextureMemoryProperties& properties, - IOSurfaceRef ioSurface); + IOSurfaceRef ioSurface, + MTLPixelFormat mtlFormat, + MTLTextureUsage mtlUsage); // Performs initialization of the base class followed by Metal-specific // initialization. MaybeError Initialize(); @@ -77,10 +79,10 @@ UnpackedPtr<EndAccessState>& state) override; MaybeError CreateMtlTextures(); - absl::InlinedVector<NSPRef<id<MTLTexture>>, kMaxPlanesPerFormat> mMtlPlaneTextures; - MTLPixelFormat mMtlFormat = MTLPixelFormatInvalid; - MTLTextureUsage mMtlUsage = MTLTextureUsageUnknown; CFRef<IOSurfaceRef> mIOSurface; + const MTLPixelFormat mMtlFormat; + const MTLTextureUsage mMtlUsage; + absl::InlinedVector<NSPRef<id<MTLTexture>>, kMaxPlanesPerFormat> mMtlPlaneTextures; }; } // namespace dawn::native::metal
diff --git a/src/dawn/native/metal/SharedTextureMemoryMTL.mm b/src/dawn/native/metal/SharedTextureMemoryMTL.mm index c8f1eed..88870bd 100644 --- a/src/dawn/native/metal/SharedTextureMemoryMTL.mm +++ b/src/dawn/native/metal/SharedTextureMemoryMTL.mm
@@ -40,13 +40,6 @@ namespace dawn::native::metal { namespace { -// NOTE: When creating MTLTextures, we pass all relevant Metal texture usages. -// See discussion in https://bugs.chromium.org/p/dawn/issues/detail?id=2152#c14 -// and following comments for both (a) why this is necessary and (b) why it is -// not harmful to performance. -const MTLTextureUsage kMetalTextureUsage = - MTLTextureUsageShaderWrite | MTLTextureUsageShaderRead | MTLTextureUsageRenderTarget; - ResultOrError<wgpu::TextureFormat> GetFormatEquivalentToIOSurfaceFormat(uint32_t format) { switch (format) { case kCVPixelFormatType_64RGBAHalf: @@ -88,6 +81,13 @@ } } +MTLTextureUsage MetalTextureUsage(bool allowStorageBinding) { + if (allowStorageBinding) { + return MTLTextureUsageShaderWrite | MTLTextureUsageShaderRead | MTLTextureUsageRenderTarget; + } + return MTLTextureUsageShaderRead | MTLTextureUsageRenderTarget; +} + } // anonymous namespace // static @@ -97,9 +97,10 @@ const SharedTextureMemoryIOSurfaceDescriptor* descriptor) { DAWN_INVALID_IF(descriptor->ioSurface == nullptr, "IOSurface is missing."); + SharedTextureMemoryProperties properties; + IOSurfaceRef ioSurface = static_cast<IOSurfaceRef>(descriptor->ioSurface); - wgpu::TextureFormat format; - DAWN_TRY_ASSIGN(format, + DAWN_TRY_ASSIGN(properties.format, GetFormatEquivalentToIOSurfaceFormat(IOSurfaceGetPixelFormat(ioSurface))); size_t width = IOSurfaceGetWidth(ioSurface); @@ -114,19 +115,30 @@ "IOSurface height (%u) exceeds maxTextureDimension2D (%u).", height, limits.v1.maxTextureDimension2D); - // IO surfaces support the following usages (the SharedTextureMemory frontend strips - // out any usages that are not supported by `format`). - const wgpu::TextureUsage kIOSurfaceSupportedUsages = - wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst | - wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::StorageBinding | - wgpu::TextureUsage::RenderAttachment; - - SharedTextureMemoryProperties properties; - properties.usage = kIOSurfaceSupportedUsages; - properties.format = format; properties.size = {static_cast<uint32_t>(width), static_cast<uint32_t>(height), 1}; - auto result = AcquireRef(new SharedTextureMemory(device, label, properties, ioSurface)); + // IO surfaces support the following usages (the SharedTextureMemory frontend strips + // out any usages that are not supported by `properties.format`). + const wgpu::TextureUsage kDefaultSupportedUsages = + wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst | + wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::RenderAttachment; + + properties.usage = kDefaultSupportedUsages; + if (descriptor->allowStorageBinding) { + properties.usage |= wgpu::TextureUsage::StorageBinding; + } + + const Format* format; + DAWN_TRY_ASSIGN(format, device->GetInternalFormat(properties.format)); + + // Multiplanar format doesn't have equivalent MTLPixelFormat so just set it to invalid. + const MTLPixelFormat mtlFormat = + format->IsMultiPlanar() ? MTLPixelFormatInvalid : MetalPixelFormat(device, format->format); + + const MTLTextureUsage mtlUsage = MetalTextureUsage(descriptor->allowStorageBinding); + + auto result = AcquireRef( + new SharedTextureMemory(device, label, properties, ioSurface, mtlFormat, mtlUsage)); DAWN_TRY(result->Initialize()); return result; @@ -135,8 +147,13 @@ SharedTextureMemory::SharedTextureMemory(Device* device, StringView label, const SharedTextureMemoryProperties& properties, - IOSurfaceRef ioSurface) - : SharedTextureMemoryBase(device, label, properties), mIOSurface(ioSurface) {} + IOSurfaceRef ioSurface, + MTLPixelFormat mtlFormat, + MTLTextureUsage mtlUsage) + : SharedTextureMemoryBase(device, label, properties), + mIOSurface(ioSurface), + mMtlFormat(mtlFormat), + mMtlUsage(mtlUsage) {} void SharedTextureMemory::DestroyImpl() { SharedTextureMemoryBase::DestroyImpl(); @@ -212,7 +229,8 @@ auto* device = static_cast<Device*>(GetDevice()); SharedTextureMemoryProperties properties; - APIGetProperties(&properties); + DAWN_TRY(GetProperties(&properties)); + const Format* format; DAWN_TRY_ASSIGN(format, device->GetInternalFormat(properties.format)); @@ -220,9 +238,6 @@ // is asked to create must be 2D/single-sampled/array length of 1/single // mipmap level. if (!format->IsMultiPlanar()) { - mMtlUsage = kMetalTextureUsage; - mMtlFormat = MetalPixelFormat(device, format->format); - // Create the descriptor for the Metal texture. auto mtlDesc = [MTLTextureDescriptor texture2DDescriptorWithPixelFormat:mMtlFormat @@ -239,10 +254,6 @@ iosurface:mIOSurface.Get() plane:0]); } else { - mMtlUsage = kMetalTextureUsage; - // Multiplanar format doesn't have equivalent MTLPixelFormat so just set it to invalid. - mMtlFormat = MTLPixelFormatInvalid; - mMtlPlaneTextures.resize(IOSurfaceGetPlaneCount(mIOSurface.Get())); for (size_t plane = 0; plane < mMtlPlaneTextures.size(); ++plane) { mMtlPlaneTextures[plane] = AcquireNSPRef(
diff --git a/src/dawn/tests/white_box/SharedTextureMemoryTests_apple.mm b/src/dawn/tests/white_box/SharedTextureMemoryTests_apple.mm index 89f25e5..a903c83 100644 --- a/src/dawn/tests/white_box/SharedTextureMemoryTests_apple.mm +++ b/src/dawn/tests/white_box/SharedTextureMemoryTests_apple.mm
@@ -34,6 +34,8 @@ #include "dawn/common/CoreFoundationRef.h" #include "dawn/common/NSRef.h" +#include "dawn/native/metal/Forward.h" +#include "dawn/native/metal/SharedTextureMemoryMTL.h" #include "dawn/tests/white_box/SharedTextureMemoryTests.h" namespace dawn { @@ -44,6 +46,25 @@ CFDictionaryAddValue(dictionary, key, number.Get()); } +wgpu::SharedTextureMemory CreateSharedTextureMemoryHelper(const wgpu::Device& device, + bool allowStorageBinding = true) { + auto dict = AcquireCFRef(CFDictionaryCreateMutable( + kCFAllocatorDefault, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks)); + AddIntegerValue(dict.Get(), kIOSurfaceWidth, 16); + AddIntegerValue(dict.Get(), kIOSurfaceHeight, 16); + AddIntegerValue(dict.Get(), kIOSurfacePixelFormat, kCVPixelFormatType_32RGBA); + AddIntegerValue(dict.Get(), kIOSurfaceBytesPerElement, 4); + + wgpu::SharedTextureMemoryIOSurfaceDescriptor ioSurfaceDesc; + ioSurfaceDesc.ioSurface = IOSurfaceCreate(dict.Get()); + ioSurfaceDesc.allowStorageBinding = allowStorageBinding; + + wgpu::SharedTextureMemoryDescriptor desc; + desc.nextInChain = &ioSurfaceDesc; + + return device.ImportSharedTextureMemory(&desc); +} + class Backend : public SharedTextureMemoryTestBackend { public: static Backend* GetInstance() { @@ -86,21 +107,7 @@ // Create one basic shared texture memory. It should support most operations. wgpu::SharedTextureMemory CreateSharedTextureMemory(const wgpu::Device& device, int layerCount) override { - auto dict = AcquireCFRef(CFDictionaryCreateMutable(kCFAllocatorDefault, 0, - &kCFTypeDictionaryKeyCallBacks, - &kCFTypeDictionaryValueCallBacks)); - AddIntegerValue(dict.Get(), kIOSurfaceWidth, 16); - AddIntegerValue(dict.Get(), kIOSurfaceHeight, 16); - AddIntegerValue(dict.Get(), kIOSurfacePixelFormat, kCVPixelFormatType_32RGBA); - AddIntegerValue(dict.Get(), kIOSurfaceBytesPerElement, 4); - - wgpu::SharedTextureMemoryIOSurfaceDescriptor ioSurfaceDesc; - ioSurfaceDesc.ioSurface = IOSurfaceCreate(dict.Get()); - - wgpu::SharedTextureMemoryDescriptor desc; - desc.nextInChain = &ioSurfaceDesc; - - return device.ImportSharedTextureMemory(&desc); + return CreateSharedTextureMemoryHelper(device); } std::vector<std::vector<wgpu::SharedTextureMemory>> CreatePerDeviceSharedTextureMemories( @@ -151,6 +158,7 @@ wgpu::SharedTextureMemoryIOSurfaceDescriptor ioSurfaceDesc; ioSurfaceDesc.ioSurface = IOSurfaceCreate(dict.Get()); + ioSurfaceDesc.allowStorageBinding = true; // Internally, the CV enums are defined as their fourcc values. Cast to that and use // it as the label. The fourcc value is a four-character name that can be @@ -278,6 +286,23 @@ ASSERT_DEVICE_ERROR(fence.ExportInfo(&exportInfo)); } +TEST_P(SharedTextureMemoryTests, DisallowStorageBinding) { + wgpu::SharedTextureMemory memory = + CreateSharedTextureMemoryHelper(device, /*allowStorageBinding=*/false); + + wgpu::SharedTextureMemoryProperties properties; + memory.GetProperties(&properties); + + EXPECT_FALSE(properties.usage & wgpu::TextureUsage::StorageBinding); + + const dawn::native::metal::SharedTextureMemory* memoryMtl = + dawn::native::metal::ToBackend(dawn::native::FromAPI(memory.Get())); + + EXPECT_FALSE(memoryMtl->GetMtlTextureUsage() & MTLTextureUsageShaderWrite); + EXPECT_TRUE(memoryMtl->GetMtlPlaneTextures()[0]); + EXPECT_EQ(memoryMtl->GetMtlPlaneTextures()[0].Get().usage, memoryMtl->GetMtlTextureUsage()); +} + DAWN_INSTANTIATE_PREFIXED_TEST_P(Metal, SharedTextureMemoryNoFeatureTests, {MetalBackend()},