Skip to content

Commit

Permalink
[WebGPU] GPUQueue.writeTexture adds layout offset twice
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=263001
<radar://116791102>

Reviewed by Tadeu Zagallo.

The offset is already added at the JS layer. We don't
need to add it again in the GPU process. It is better
to add any offsets at the JS layer because less data
needs to be sent from the web content process to the gpu
process.

* Source/WebGPU/WebGPU/Queue.mm:
(WebGPU::Queue::writeTexture):

Canonical link: https://commits.webkit.org/269237@main
  • Loading branch information
mwyrzykowski committed Oct 12, 2023
1 parent 5be53a0 commit 1fea69c
Showing 1 changed file with 6 additions and 9 deletions.
15 changes: 6 additions & 9 deletions Source/WebGPU/WebGPU/Queue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,9 @@ static bool validateWriteTexture(const WGPUImageCopyTexture& destination, const

// https://gpuweb.github.io/gpuweb/#dom-gpuqueue-writetexture

auto dataByteSize = dataSize;

const auto& texture = fromAPI(destination.texture);

if (!validateWriteTexture(destination, dataLayout, size, dataByteSize, texture)) {
if (!validateWriteTexture(destination, dataLayout, size, dataSize, texture)) {
m_device.generateAValidationError("Validation failure."_s);
return;
}
Expand Down Expand Up @@ -355,7 +353,7 @@ static bool validateWriteTexture(const WGPUImageCopyTexture& destination, const
case WGPUTextureDimension_1D: {
auto region = MTLRegionMake1D(destination.origin.x, size.width);
for (uint32_t layer = 0; layer < size.depthOrArrayLayers; ++layer) {
auto sourceOffset = static_cast<NSUInteger>(dataLayout.offset + layer * bytesPerImage);
auto sourceOffset = static_cast<NSUInteger>(layer * bytesPerImage);
NSUInteger destinationSlice = destination.origin.z + layer;
[fromAPI(destination.texture).texture()
replaceRegion:region
Expand All @@ -370,7 +368,7 @@ static bool validateWriteTexture(const WGPUImageCopyTexture& destination, const
case WGPUTextureDimension_2D: {
auto region = MTLRegionMake2D(destination.origin.x, destination.origin.y, size.width, size.height);
for (uint32_t layer = 0; layer < size.depthOrArrayLayers; ++layer) {
auto sourceOffset = static_cast<NSUInteger>(dataLayout.offset + layer * bytesPerImage);
auto sourceOffset = static_cast<NSUInteger>(layer * bytesPerImage);
NSUInteger destinationSlice = destination.origin.z + layer;
[fromAPI(destination.texture).texture()
replaceRegion:region
Expand All @@ -384,12 +382,11 @@ static bool validateWriteTexture(const WGPUImageCopyTexture& destination, const
}
case WGPUTextureDimension_3D: {
auto region = MTLRegionMake3D(destination.origin.x, destination.origin.y, destination.origin.z, size.width, size.height, size.depthOrArrayLayers);
auto sourceOffset = static_cast<NSUInteger>(dataLayout.offset);
[fromAPI(destination.texture).texture()
replaceRegion:region
mipmapLevel:destination.mipLevel
slice:0
withBytes:static_cast<const char*>(data) + sourceOffset
withBytes:static_cast<const char*>(data)
bytesPerRow:bytesPerRow
bytesPerImage:bytesPerImage];
break;
Expand All @@ -409,13 +406,13 @@ static bool validateWriteTexture(const WGPUImageCopyTexture& destination, const
}
}

ensureBlitCommandEncoder();
// FIXME(PERFORMANCE): Suballocate, so the common case doesn't need to hit the kernel.
// FIXME(PERFORMANCE): Should this temporary buffer really be shared?
id<MTLBuffer> temporaryBuffer = [m_device.device() newBufferWithBytes:static_cast<const char*>(data) + dataLayout.offset length:static_cast<NSUInteger>(dataByteSize) options:MTLResourceStorageModeShared];
id<MTLBuffer> temporaryBuffer = [m_device.device() newBufferWithBytes:static_cast<const char*>(data) length:static_cast<NSUInteger>(dataSize) options:MTLResourceStorageModeShared];
if (!temporaryBuffer)
return;

ensureBlitCommandEncoder();
auto logicalSize = fromAPI(destination.texture).logicalMiplevelSpecificTextureExtent(destination.mipLevel);
auto widthForMetal = std::min(size.width, logicalSize.width);
auto heightForMetal = std::min(size.height, logicalSize.height);
Expand Down

0 comments on commit 1fea69c

Please sign in to comment.