Skip to content

Commit

Permalink
Unreviewed, reverting 278474@main (36690be)
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=273973
<radar://127121787>

This caused a regression in some CTS tests

Reverted change:

    [WebGPU] Invalid command buffers need to get dealloc'ed in a timely fashion
    https://bugs.webkit.org/show_bug.cgi?id=273328
    278474@main (36690be)

Canonical link: https://commits.webkit.org/278594@main
  • Loading branch information
mwyrzykowski committed May 9, 2024
1 parent 98c3511 commit 8fd6dde
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 39 deletions.
7 changes: 3 additions & 4 deletions Source/WebGPU/WebGPU/CommandBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ class Device;
class CommandBuffer : public WGPUCommandBufferImpl, public RefCounted<CommandBuffer>, public CanMakeWeakPtr<CommandBuffer> {
WTF_MAKE_FAST_ALLOCATED;
public:
static Ref<CommandBuffer> create(id<MTLCommandBuffer> commandBuffer, id<MTLSharedEvent> event, Device& device)
static Ref<CommandBuffer> create(id<MTLCommandBuffer> commandBuffer, Device& device)
{
return adoptRef(*new CommandBuffer(commandBuffer, event, device));
return adoptRef(*new CommandBuffer(commandBuffer, device));
}
static Ref<CommandBuffer> createInvalid(Device& device)
{
Expand All @@ -68,11 +68,10 @@ class CommandBuffer : public WGPUCommandBufferImpl, public RefCounted<CommandBuf
void waitForCompletion();

private:
CommandBuffer(id<MTLCommandBuffer>, id<MTLSharedEvent>, Device&);
CommandBuffer(id<MTLCommandBuffer>, Device&);
CommandBuffer(Device&);

id<MTLCommandBuffer> m_commandBuffer { nil };
id<MTLSharedEvent> m_abortEvent { nil };
id<MTLCommandBuffer> m_cachedCommandBuffer { nil };
int m_bufferMapCount { 0 };

Expand Down
4 changes: 1 addition & 3 deletions Source/WebGPU/WebGPU/CommandBuffer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@

namespace WebGPU {

CommandBuffer::CommandBuffer(id<MTLCommandBuffer> commandBuffer, id<MTLSharedEvent> event, Device& device)
CommandBuffer::CommandBuffer(id<MTLCommandBuffer> commandBuffer, Device& device)
: m_commandBuffer(commandBuffer)
, m_abortEvent(event)
, m_device(device)
{
}
Expand All @@ -51,7 +50,6 @@

void CommandBuffer::makeInvalid(NSString* lastError)
{
[m_abortEvent setSignaledValue:1];
m_lastErrorString = lastError;
m_device->getQueue().commitMTLCommandBuffer(m_commandBuffer);
m_commandBuffer = nil;
Expand Down
7 changes: 3 additions & 4 deletions Source/WebGPU/WebGPU/CommandEncoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ class Texture;
class CommandEncoder : public WGPUCommandEncoderImpl, public RefCounted<CommandEncoder>, public CommandsMixin, public CanMakeWeakPtr<CommandEncoder> {
WTF_MAKE_FAST_ALLOCATED;
public:
static Ref<CommandEncoder> create(id<MTLCommandBuffer> commandBuffer, id<MTLSharedEvent> event, Device& device)
static Ref<CommandEncoder> create(id<MTLCommandBuffer> commandBuffer, Device& device)
{
return adoptRef(*new CommandEncoder(commandBuffer, event, device));
return adoptRef(*new CommandEncoder(commandBuffer, device));
}
static Ref<CommandEncoder> createInvalid(Device& device)
{
Expand Down Expand Up @@ -106,7 +106,7 @@ class CommandEncoder : public WGPUCommandEncoderImpl, public RefCounted<CommandE
bool encoderIsCurrent(id<MTLCommandEncoder>) const;

private:
CommandEncoder(id<MTLCommandBuffer>, id<MTLSharedEvent>, Device&);
CommandEncoder(id<MTLCommandBuffer>, Device&);
CommandEncoder(Device&);

NSString* errorValidatingCopyBufferToBuffer(const Buffer& source, uint64_t sourceOffset, const Buffer& destination, uint64_t destinationOffset, uint64_t size);
Expand All @@ -123,7 +123,6 @@ class CommandEncoder : public WGPUCommandEncoderImpl, public RefCounted<CommandE
NSString* errorValidatingCopyTextureToBuffer(const WGPUImageCopyTexture&, const WGPUImageCopyBuffer&, const WGPUExtent3D&) const;

id<MTLCommandBuffer> m_commandBuffer { nil };
id<MTLSharedEvent> m_abortCommandBuffer { nil };
id<MTLBlitCommandEncoder> m_blitCommandEncoder { nil };
id<MTLCommandEncoder> m_existingCommandEncoder { nil };
struct PendingTimestampWrites {
Expand Down
15 changes: 6 additions & 9 deletions Source/WebGPU/WebGPU/CommandEncoder.mm
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,17 @@ static MTLStoreAction storeAction(WGPUStoreOp storeOp, bool hasResolveTarget = f

auto *commandBufferDescriptor = [MTLCommandBufferDescriptor new];
commandBufferDescriptor.errorOptions = MTLCommandBufferErrorOptionEncoderExecutionStatus;
auto commandBufferWithEvent = getQueue().commandBufferWithDescriptor(commandBufferDescriptor);
if (!commandBufferWithEvent.first)
id<MTLCommandBuffer> commandBuffer = getQueue().commandBufferWithDescriptor(commandBufferDescriptor);
if (!commandBuffer)
return CommandEncoder::createInvalid(*this);

commandBufferWithEvent.first.label = fromAPI(descriptor.label);
commandBuffer.label = fromAPI(descriptor.label);

return CommandEncoder::create(commandBufferWithEvent.first, commandBufferWithEvent.second, *this);
return CommandEncoder::create(commandBuffer, *this);
}

CommandEncoder::CommandEncoder(id<MTLCommandBuffer> commandBuffer, id<MTLSharedEvent> event, Device& device)
CommandEncoder::CommandEncoder(id<MTLCommandBuffer> commandBuffer, Device& device)
: m_commandBuffer(commandBuffer)
, m_abortCommandBuffer(event)
, m_device(device)
{
}
Expand Down Expand Up @@ -1201,7 +1200,6 @@ static bool refersToAllAspects(WGPUTextureFormat format, WGPUTextureAspect aspec
{
endEncoding(m_existingCommandEncoder);
m_blitCommandEncoder = nil;
[m_abortCommandBuffer setSignaledValue:1];
m_device->getQueue().commitMTLCommandBuffer(m_commandBuffer);

m_commandBuffer = nil;
Expand Down Expand Up @@ -1745,8 +1743,7 @@ static bool areCopyCompatible(WGPUTextureFormat format1, WGPUTextureFormat forma

commandBuffer.label = fromAPI(descriptor.label);

auto result = CommandBuffer::create(commandBuffer, m_abortCommandBuffer, m_device);
m_abortCommandBuffer = nil;
auto result = CommandBuffer::create(commandBuffer, m_device);
m_cachedCommandBuffer = result;
m_cachedCommandBuffer->setBufferMapCount(m_bufferMapCount);
if (m_makeSubmitInvalid)
Expand Down
4 changes: 0 additions & 4 deletions Source/WebGPU/WebGPU/MetalSPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,10 @@
*/

#if USE(INTERNAL_APPLE_SDK)
#import <Metal/MTLCommandBuffer_Private.h>
#import <Metal/MTLResource_Private.h>
#else
@protocol MTLResourceSPI <MTLResource>
@optional
- (kern_return_t)setOwnerWithIdentity:(mach_port_t)task_id_token;
@end
@protocol MTLCommandBufferSPI <MTLCommandBuffer>
- (void)encodeConditionalAbortEvent:(id <MTLSharedEvent>)event;
@end
#endif
2 changes: 1 addition & 1 deletion Source/WebGPU/WebGPU/PresentationContextIOSurface.mm
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ kernel void luminanceClamp(texture2d<float, access::read> inTexture [[texture(
if (Texture* texturePtr = textureRefPtr.get(); texturePtr && m_computePipelineState) {
MTLCommandBufferDescriptor *descriptor = [MTLCommandBufferDescriptor new];
descriptor.errorOptions = MTLCommandBufferErrorOptionEncoderExecutionStatus;
id<MTLCommandBuffer> commandBuffer = m_device->getQueue().commandBufferWithDescriptor(descriptor).first;
id<MTLCommandBuffer> commandBuffer = m_device->getQueue().commandBufferWithDescriptor(descriptor);
MTLComputePassDescriptor* computeDescriptor = [MTLComputePassDescriptor new];
computeDescriptor.dispatchType = MTLDispatchTypeSerial;

Expand Down
3 changes: 1 addition & 2 deletions Source/WebGPU/WebGPU/Queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class Queue : public WGPUQueueImpl, public ThreadSafeRefCounted<Queue> {

const Device& device() const;
void clearTextureIfNeeded(const WGPUImageCopyTexture&, NSUInteger);
std::pair<id<MTLCommandBuffer>, id<MTLSharedEvent>> commandBufferWithDescriptor(MTLCommandBufferDescriptor*);
id<MTLCommandBuffer> commandBufferWithDescriptor(MTLCommandBufferDescriptor*);
void commitMTLCommandBuffer(id<MTLCommandBuffer>);
void setEncoderForBuffer(id<MTLCommandBuffer>, id<MTLCommandEncoder>);
id<MTLCommandEncoder> encoderForBuffer(id<MTLCommandBuffer>) const;
Expand All @@ -101,7 +101,6 @@ class Queue : public WGPUQueueImpl, public ThreadSafeRefCounted<Queue> {

id<MTLCommandQueue> m_commandQueue { nil };
id<MTLCommandBuffer> m_commandBuffer { nil };
id<MTLSharedEvent> m_commandBufferEvent { nil };
id<MTLBlitCommandEncoder> m_blitCommandEncoder { nil };
ThreadSafeWeakPtr<Device> m_device; // The only kind of queues that exist right now are default queues, which are owned by Devices.

Expand Down
15 changes: 3 additions & 12 deletions Source/WebGPU/WebGPU/Queue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#import "CommandEncoder.h"
#import "Device.h"
#import "IsValidToUseWith.h"
#import "MetalSPI.h"
#import "Texture.h"
#import "TextureView.h"
#import <wtf/CheckedArithmetic.h>
Expand Down Expand Up @@ -71,9 +70,7 @@

auto *commandBufferDescriptor = [MTLCommandBufferDescriptor new];
commandBufferDescriptor.errorOptions = MTLCommandBufferErrorOptionEncoderExecutionStatus;
auto blitCommandBufferWithSharedEvent = commandBufferWithDescriptor(commandBufferDescriptor);
m_commandBuffer = blitCommandBufferWithSharedEvent.first;
m_commandBufferEvent = blitCommandBufferWithSharedEvent.second;
m_commandBuffer = commandBufferWithDescriptor(commandBufferDescriptor);
m_blitCommandEncoder = [m_commandBuffer blitCommandEncoder];
}

Expand Down Expand Up @@ -106,7 +103,7 @@
[m_openCommandEncoders setObject:commandEncoder forKey:commandBuffer];
}

std::pair<id<MTLCommandBuffer>, id<MTLSharedEvent>> Queue::commandBufferWithDescriptor(MTLCommandBufferDescriptor* descriptor)
id<MTLCommandBuffer> Queue::commandBufferWithDescriptor(MTLCommandBufferDescriptor* descriptor)
{
constexpr auto maxCommandBufferCount = 64;
if (m_createdNotCommittedBuffers.count >= maxCommandBufferCount) {
Expand All @@ -121,14 +118,8 @@
id<MTLCommandBuffer> buffer = [m_commandQueue commandBufferWithDescriptor:descriptor];
if (buffer)
[m_createdNotCommittedBuffers addObject:buffer];
auto devicePtr = m_device.get();
id<MTLSharedEvent> sharedEvent = nil;
if (devicePtr && [buffer respondsToSelector:@selector(encodeConditionalAbortEvent:)]) {
sharedEvent = [devicePtr->device() newSharedEvent];
[(id<MTLCommandBufferSPI>)buffer encodeConditionalAbortEvent:sharedEvent];
}

return std::make_pair(buffer, sharedEvent);
return buffer;
}

void Queue::makeInvalid()
Expand Down

0 comments on commit 8fd6dde

Please sign in to comment.