Skip to content

Commit 04cbd20

Browse files
gowtham-sarcAngle LUCI CQ
gowtham-sarc
authored and
Angle LUCI CQ
committed
CL/Vulkan: Fix the rect size calculation
Take into consideration on the extraneous region when calculating the size of the enclosing buffer size for buffer rects. Bug: angleproject:444481344 Change-Id: Ifd146035565abf5bea0650ccee375bc2b28a55c1 Signed-off-by: Gowtham Tammana <g.tammana@samsung.com> Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6939055 Commit-Queue: Austin Annestrand <a.annestrand@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org>
1 parent b1fc47c commit 04cbd20

4 files changed

Lines changed: 117 additions & 59 deletions

File tree

‎src/libANGLE/cl_types.h‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,56 @@ struct BufferRect
120120
return ((mRowPitch * (mOrigin.y + row)) + (mOrigin.x * mElementSize)) + // row offset
121121
(mSlicePitch * (mOrigin.z + slice)); // slice offset
122122
}
123+
// Calculates the offset of the starting position of the rectangle
124+
size_t getBufferOffset() const { return getRowOffset(0, 0); }
123125

124126
size_t getRowPitch() const { return mRowPitch; }
125127
size_t getSlicePitch() const { return mSlicePitch; }
128+
126129
// Given the offset, row pitch, slice pitch, this returns the size of the buffer in which this
127130
// rect sits.
128-
size_t getRectSize() const { return getRowOffset(mSize.depth, mSize.height); }
131+
//
132+
// row_pitch
133+
// +------------------------------+
134+
// | origin |
135+
// | +---------------+ |
136+
// | | |height |
137+
// | | | |
138+
// | | width | |
139+
// +------+---------------+-------+
140+
// - size of the buffer is
141+
// - initial offset to start of rect
142+
// - + size of the rectangle
143+
// - - (extra regions for the last slice and row)
144+
size_t getRectSize() const
145+
{
146+
// Treat:
147+
// S: mSlicePitch, R: mRowPitch, O: Offset(xyz_dim), D: mSize.depth, H: mSize.height,
148+
// W: mSize.width, E: mElementSize
149+
//
150+
// strideHeightPadding == (S / R) - H
151+
// strideRowPadding == R - (W * E)
152+
//
153+
// getRectSize() =
154+
// getBufferOffset() // initial offset to the start of rect
155+
// + (S * D) // size of the rectangle
156+
// - (strideHeightPadding * R) // extraneous region for the last slice
157+
// - strideRowPadding // extraneous region for the last row
158+
//
159+
// Where:
160+
// getBufferOffset() == getRowOffset(0,0) == S(Oz) + R(Oy) + (Ox * E)
161+
// getRowOffset(s, r) == S(Oz + s) + R(Oy + r) + (Ox * E)
162+
//
163+
// Simplifies to:
164+
// getRectSize() = S(Oz) + R(Oy) + (Ox * E) + (S * D) - R(S/R - H) - R - (W * E)
165+
// getRectSize() = S(Oz + (D - 1)) + R(Oy + (H - 1)) + (Ox * E) + (W * E)
166+
// ...
167+
// getRectSize() = getRowOffset((D - 1), (H - 1)) + (W * E)
168+
169+
// The end of the rect is the beginning of the last row + the length of the row.
170+
// This logic excludes paddings for the last row / slice.
171+
return getRowOffset(mSize.depth - 1, mSize.height - 1) + mSize.width * mElementSize;
172+
}
129173
const Extents &getExtents() const { return mSize; }
130174
Offset mOrigin;
131175
Extents mSize;

‎src/libANGLE/renderer/vulkan/CLCommandQueueVk.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,10 @@ class HostTransferConfigVisitor
182182
switch (transferConfig.getType())
183183
{
184184
case CL_COMMAND_READ_BUFFER:
185-
ANGLE_TRY(bufferVk.syncHost(CLBufferVk::SyncHostDirection::ToHost,
186-
transferConfig.getOffset(), transferConfig.getSize()));
185+
ANGLE_TRY(bufferVk.syncHost(CLBufferVk::SyncHostDirection::ToHost));
187186
break;
188187
case CL_COMMAND_READ_BUFFER_RECT:
189188
ANGLE_TRY(bufferVk.syncHost(CLBufferVk::SyncHostDirection::ToHost,
190-
transferConfig.getBufferRect(),
191189
transferConfig.getHostRect()));
192190
break;
193191
case CL_COMMAND_READ_IMAGE:
@@ -456,7 +454,7 @@ angle::Result CLCommandQueueVk::enqueueWriteBufferRect(const cl::Buffer &buffer,
456454
{
457455
// Stage a transfer routine
458456
HostWriteTransferConfig config(CL_COMMAND_WRITE_BUFFER_RECT, ptrRect.getRectSize(),
459-
const_cast<void *>(ptr), ptrRect, bufferRect);
457+
const_cast<void *>(ptr), bufferRect, ptrRect);
460458
ANGLE_TRY(addToHostTransferList(bufferVk, config));
461459
}
462460

@@ -774,7 +772,7 @@ angle::Result CLCommandQueueVk::addToHostTransferList(CLBufferVk *srcBuffer,
774772
}
775773
case CL_COMMAND_READ_BUFFER:
776774
{
777-
VkBufferCopy copyRegion = {0, transferConfig.getOffset(), transferConfig.getSize()};
775+
VkBufferCopy copyRegion = {transferConfig.getOffset(), 0, transferConfig.getSize()};
778776
copyRegion.srcOffset += srcBuffer->getOffset();
779777
copyRegion.dstOffset += transferBufferHandleVk.getOffset();
780778
mComputePassCommands->getCommandBuffer().copyBuffer(

‎src/libANGLE/renderer/vulkan/CLMemoryVk.cpp‎

Lines changed: 54 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -259,9 +259,9 @@ vk::BufferHelper &CLBufferVk::getBuffer()
259259
return mBuffer;
260260
}
261261

262-
angle::Result CLBufferVk::syncHost(CLBufferVk::SyncHostDirection direction,
263-
size_t offset,
264-
size_t size)
262+
// For UHP buffers, the buffer contents and hostptr have to be in sync at appropriate times. Ensure
263+
// that if zero copy is not supported.
264+
angle::Result CLBufferVk::syncHost(CLBufferVk::SyncHostDirection direction)
265265
{
266266
switch (direction)
267267
{
@@ -276,7 +276,7 @@ angle::Result CLBufferVk::syncHost(CLBufferVk::SyncHostDirection direction,
276276
case CLBufferVk::SyncHostDirection::ToHost:
277277
if (getFlags().intersects(CL_MEM_USE_HOST_PTR) && !supportsZeroCopy())
278278
{
279-
ANGLE_TRY(copyTo(getHostPtr(), offset, size));
279+
ANGLE_TRY(copyTo(getHostPtr(), 0, getSize()));
280280
}
281281
break;
282282
default:
@@ -286,22 +286,22 @@ angle::Result CLBufferVk::syncHost(CLBufferVk::SyncHostDirection direction,
286286
return angle::Result::Continue;
287287
}
288288

289-
angle::Result CLBufferVk::syncHost(CLBufferVk::SyncHostDirection direction,
290-
cl::BufferRect bufferRect,
291-
cl::BufferRect hostRect)
289+
// This is to sync only a rectangular region between hostptr and buffer contents. Intended to be
290+
// used for READ/WRITE_RECT.
291+
angle::Result CLBufferVk::syncHost(CLBufferVk::SyncHostDirection direction, cl::BufferRect hostRect)
292292
{
293293
switch (direction)
294294
{
295295
case CLBufferVk::SyncHostDirection::FromHost:
296296
if (getFlags().intersects(CL_MEM_USE_HOST_PTR) && !supportsZeroCopy())
297297
{
298-
ANGLE_TRY(setRect(getHostPtr(), bufferRect, hostRect));
298+
ANGLE_TRY(setRect(getHostPtr(), hostRect, hostRect));
299299
}
300300
break;
301301
case CLBufferVk::SyncHostDirection::ToHost:
302302
if (getFlags().intersects(CL_MEM_USE_HOST_PTR) && !supportsZeroCopy())
303303
{
304-
ANGLE_TRY(getRect(bufferRect, hostRect, getHostPtr()));
304+
ANGLE_TRY(getRect(hostRect, hostRect, getHostPtr()));
305305
}
306306
break;
307307
default:
@@ -342,7 +342,7 @@ angle::Result CLBufferVk::create(void *hostPtr)
342342
ANGLE_CL_IMPL_TRY_ERROR(setDataImpl(static_cast<uint8_t *>(hostPtr), getSize(), 0),
343343
CL_OUT_OF_RESOURCES);
344344
}
345-
ANGLE_TRY(syncHost(CLBufferVk::SyncHostDirection::FromHost, 0, getSize()));
345+
ANGLE_TRY(syncHost(CLBufferVk::SyncHostDirection::FromHost));
346346
}
347347
return angle::Result::Continue;
348348
}
@@ -455,56 +455,64 @@ void CLBufferVk::unmapBufferHelper()
455455
mMappedMemory = nullptr;
456456
}
457457

458-
angle::Result CLBufferVk::setRect(const void *data,
459-
const cl::BufferRect &srcRect,
460-
const cl::BufferRect &rect)
458+
angle::Result CLBufferVk::updateRect(UpdateRectOperation op,
459+
void *data,
460+
const cl::BufferRect &dataRect,
461+
const cl::BufferRect &bufferRect)
461462
{
462-
ASSERT(srcRect.valid() && rect.valid());
463-
ASSERT(srcRect.mSize == rect.mSize);
463+
ASSERT(dataRect.valid() && bufferRect.valid());
464+
ASSERT(dataRect.mSize == bufferRect.mSize && dataRect.mElementSize == bufferRect.mElementSize);
464465

465-
uint8_t *mapPtr = nullptr;
466-
ANGLE_TRY(mapBufferHelper(mapPtr));
466+
uint8_t *bufferPtr = nullptr;
467+
ANGLE_TRY(mapBufferHelper(bufferPtr));
467468
cl::Defer deferUnmap([this]() { unmapBufferHelper(); });
468469

469-
const uint8_t *srcData = reinterpret_cast<const uint8_t *>(data);
470-
for (size_t slice = 0; slice < rect.mSize.depth; slice++)
470+
uint8_t *dataUint8Ptr = reinterpret_cast<uint8_t *>(data);
471+
for (size_t slice = 0; slice < bufferRect.mSize.depth; slice++)
471472
{
472-
for (size_t row = 0; row < rect.mSize.height; row++)
473+
for (size_t row = 0; row < bufferRect.mSize.height; row++)
473474
{
474-
const uint8_t *src = srcData + srcRect.getRowOffset(slice, row);
475-
uint8_t *dst = mapPtr + rect.getRowOffset(slice, row);
475+
uint8_t *offsetDataPtr = dataUint8Ptr + dataRect.getRowOffset(slice, row);
476+
uint8_t *offsetBufferPtr = bufferPtr + bufferRect.getRowOffset(slice, row);
477+
size_t updateSize = dataRect.mSize.width * dataRect.mElementSize;
476478

477-
memcpy(dst, src, srcRect.mSize.width * srcRect.mElementSize);
479+
switch (op)
480+
{
481+
case UpdateRectOperation::Read:
482+
{
483+
// Read from this buffer
484+
memcpy(offsetDataPtr, offsetBufferPtr, updateSize);
485+
break;
486+
}
487+
case UpdateRectOperation::Write:
488+
{
489+
// Write to this buffer
490+
memcpy(offsetBufferPtr, offsetDataPtr, updateSize);
491+
break;
492+
}
493+
default:
494+
{
495+
UNREACHABLE();
496+
break;
497+
}
498+
}
478499
}
479500
}
480-
481501
return angle::Result::Continue;
482502
}
483503

484-
angle::Result CLBufferVk::getRect(const cl::BufferRect &srcRect,
485-
const cl::BufferRect &outRect,
486-
void *outData)
504+
angle::Result CLBufferVk::setRect(const void *data,
505+
const cl::BufferRect &dataRect,
506+
const cl::BufferRect &bufferRect)
487507
{
488-
ASSERT(srcRect.valid() && outRect.valid());
489-
ASSERT(srcRect.mSize == outRect.mSize);
490-
491-
uint8_t *mapPtr = nullptr;
492-
ANGLE_TRY(mapBufferHelper(mapPtr));
493-
cl::Defer deferUnmap([this]() { unmapBufferHelper(); });
494-
495-
uint8_t *dstData = reinterpret_cast<uint8_t *>(outData);
496-
for (size_t slice = 0; slice < srcRect.mSize.depth; slice++)
497-
{
498-
for (size_t row = 0; row < srcRect.mSize.height; row++)
499-
{
500-
const uint8_t *src = mapPtr + srcRect.getRowOffset(slice, row);
501-
uint8_t *dst = dstData + outRect.getRowOffset(slice, row);
502-
503-
memcpy(dst, src, srcRect.mSize.width * srcRect.mElementSize);
504-
}
505-
}
508+
return updateRect(UpdateRectOperation::Write, const_cast<void *>(data), dataRect, bufferRect);
509+
}
506510

507-
return angle::Result::Continue;
511+
angle::Result CLBufferVk::getRect(const cl::BufferRect &bufferRect,
512+
const cl::BufferRect &dataRect,
513+
void *outData)
514+
{
515+
return updateRect(UpdateRectOperation::Read, outData, dataRect, bufferRect);
508516
}
509517

510518
// offset is for mapped pointer

‎src/libANGLE/renderer/vulkan/CLMemoryVk.h‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,10 @@ class CLBufferVk : public CLMemoryVk
125125
bool isSubBuffer() const { return mParent != nullptr; }
126126

127127
angle::Result setRect(const void *data,
128-
const cl::BufferRect &srcRect,
128+
const cl::BufferRect &dataRect,
129129
const cl::BufferRect &bufferRect);
130-
angle::Result getRect(const cl::BufferRect &srcRect,
131-
const cl::BufferRect &outRect,
130+
angle::Result getRect(const cl::BufferRect &bufferRect,
131+
const cl::BufferRect &dataRect,
132132
void *outData);
133133

134134
bool isCurrentlyInUse() const override;
@@ -140,10 +140,8 @@ class CLBufferVk : public CLMemoryVk
140140
ToHost,
141141
FromHost
142142
};
143-
angle::Result syncHost(CLBufferVk::SyncHostDirection direction, size_t offset, size_t size);
144-
angle::Result syncHost(CLBufferVk::SyncHostDirection direction,
145-
cl::BufferRect bufferRect,
146-
cl::BufferRect hostRect);
143+
angle::Result syncHost(CLBufferVk::SyncHostDirection direction);
144+
angle::Result syncHost(CLBufferVk::SyncHostDirection direction, cl::BufferRect hostRect);
147145

148146
private:
149147
angle::Result mapBufferHelper(uint8_t *&ptrOut) override;
@@ -152,6 +150,16 @@ class CLBufferVk : public CLMemoryVk
152150
angle::Result setDataImpl(const uint8_t *data, size_t size, size_t offset);
153151
angle::Result createWithProperties();
154152

153+
enum class UpdateRectOperation
154+
{
155+
Read,
156+
Write
157+
};
158+
angle::Result updateRect(UpdateRectOperation readWriteOp,
159+
void *data,
160+
const cl::BufferRect &dataRect,
161+
const cl::BufferRect &bufferRect);
162+
155163
vk::BufferHelper mBuffer;
156164
VkBufferCreateInfo mDefaultBufferCreateInfo;
157165

0 commit comments

Comments
 (0)