1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

rhi: add QByteArray overloads avoiding copies to QRhiResourceUpdateBatch

Browse Source 
It seems wasteful to make a copy of the buffer we want to upload inside
the update batch, especially when we construct a large geometry inside a
QByteArray in user code.

Avoid such copies, by creating update/upload overloads accepting a
QByteArray. The received QByteArray is moved inside largeData directly,
instead of allocating a new char[size] and then memcpying into it.

The QByteArray is still copied into smallData if its size is less than
SMALL_DATA_SIZE. This patch only applies to large buffers.

Modify 2 auto tests to use the new uploadStaticBuffer() overload.

[ChangeLog][RHI] Added QByteArray overloads to
QRhiResourceUpdateBatch::updateDynamicBuffer and
QRhiResourceUpdateBatch::uploadStaticBuffer that allow passing large
buffers without copy to the RHI.

Change-Id: I58f41b2f7889d88b565237819c0dcecf90988cb2
Reviewed-by: Laszlo Agocs <laszlo.agocs@qt.io>
This commit is contained in:
Aurélien Brooke 2025-01-23 09:59:37 +01:00
parent 38fc8d845d
commit 5d857ed3bc
4 changed files with 132 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
src/gui/rhi/qrhi.cpp
View File

@ -9479,7 +9479,9 @@ bool QRhiResourceUpdateBatch::hasOptimalCapacity() const
The region is specified \a offset and \a size. The actual bytes to write
are specified by \a data which must have at least \a size bytes available.
\a data can safely be destroyed or changed once this function returns.
\a data is copied and can safely be destroyed or changed once this function
returns.
\note If host writes are involved, which is the case with
updateDynamicBuffer() typically as such buffers are backed by host visible
@ -9504,13 +9506,36 @@ void QRhiResourceUpdateBatch::updateDynamicBuffer(QRhiBuffer *buf, quint32 offse
}
}
/*!
\overload
\since 6.10
Enqueues updating a region of a QRhiBuffer \a buf created with the type
QRhiBuffer::Dynamic.
\a data is moved into the batch instead of copied with this overload.
*/
void QRhiResourceUpdateBatch::updateDynamicBuffer(QRhiBuffer *buf, quint32 offset, QByteArray data)
{
if (!data.isEmpty()) {
const int idx = d->activeBufferOpCount++;
const int opListSize = d->bufferOps.size();
if (idx < opListSize)
QRhiResourceUpdateBatchPrivate::BufferOp::changeToDynamicUpdate(&d->bufferOps[idx], buf, offset, std::move(data));
else
d->bufferOps.append(QRhiResourceUpdateBatchPrivate::BufferOp::dynamicUpdate(buf, offset, std::move(data)));
}
}
/*!
Enqueues updating a region of a QRhiBuffer \a buf created with the type
QRhiBuffer::Immutable or QRhiBuffer::Static.
The region is specified \a offset and \a size. The actual bytes to write
are specified by \a data which must have at least \a size bytes available.
\a data can safely be destroyed or changed once this function returns.
\a data is copied and can safely be destroyed or changed once this function
returns.
*/
void QRhiResourceUpdateBatch::uploadStaticBuffer(QRhiBuffer *buf, quint32 offset, quint32 size, const void *data)
{
@ -9523,6 +9548,26 @@ void QRhiResourceUpdateBatch::uploadStaticBuffer(QRhiBuffer *buf, quint32 offset
}
}
/*!
\overload
\since 6.10
Enqueues updating a region of a QRhiBuffer \a buf created with the type
QRhiBuffer::Immutable or QRhiBuffer::Static.
\a data is moved into the batch instead of copied with this overload.
*/
void QRhiResourceUpdateBatch::uploadStaticBuffer(QRhiBuffer *buf, quint32 offset, QByteArray data)
{
if (!data.isEmpty()) {
const int idx = d->activeBufferOpCount++;
if (idx < d->bufferOps.size())
QRhiResourceUpdateBatchPrivate::BufferOp::changeToStaticUpload(&d->bufferOps[idx], buf, offset, std::move(data));
else
d->bufferOps.append(QRhiResourceUpdateBatchPrivate::BufferOp::staticUpload(buf, offset, std::move(data)));
}
}
/*!
\overload
@ -9540,6 +9585,28 @@ void QRhiResourceUpdateBatch::uploadStaticBuffer(QRhiBuffer *buf, const void *da
}
}
/*!
\overload
\since 6.10
Enqueues updating the entire QRhiBuffer \a buf created with the type
QRhiBuffer::Immutable or QRhiBuffer::Static.
\a data is moved into the batch instead of copied with this overload.
\a data size must equal the size of \a buf.
*/
void QRhiResourceUpdateBatch::uploadStaticBuffer(QRhiBuffer *buf, QByteArray data)
{
if (buf->size() > 0 && quint32(data.size()) == buf->size()) {
const int idx = d->activeBufferOpCount++;
if (idx < d->bufferOps.size())
QRhiResourceUpdateBatchPrivate::BufferOp::changeToStaticUpload(&d->bufferOps[idx], buf, 0, 0, std::move(data));
else
d->bufferOps.append(QRhiResourceUpdateBatchPrivate::BufferOp::staticUpload(buf, 0, 0, std::move(data)));
}
}
/*!
Enqueues reading back a region of the QRhiBuffer \a buf. The size of the
region is specified by \a size in bytes, \a offset is the offset in bytes

3
src/gui/rhi/qrhi.h
View File

@ -1774,8 +1774,11 @@ public:
bool hasOptimalCapacity() const;
void updateDynamicBuffer(QRhiBuffer *buf, quint32 offset, quint32 size, const void *data);
void updateDynamicBuffer(QRhiBuffer *buf, quint32 offset, QByteArray data);
void uploadStaticBuffer(QRhiBuffer *buf, quint32 offset, quint32 size, const void *data);
void uploadStaticBuffer(QRhiBuffer *buf, quint32 offset, QByteArray data);
void uploadStaticBuffer(QRhiBuffer *buf, const void *data);
void uploadStaticBuffer(QRhiBuffer *buf, QByteArray data);
void readBackBuffer(QRhiBuffer *buf, quint32 offset, quint32 size, QRhiReadbackResult *result);
void uploadTexture(QRhiTexture *tex, const QRhiTextureUploadDescription &desc);
void uploadTexture(QRhiTexture *tex, const QImage &image);

72
src/gui/rhi/qrhi_p.h
View File

@ -324,14 +324,12 @@ bool qrhi_toTopLeftRenderTargetRect(const QSize &outputSize, const std::array<T,
struct QRhiBufferDataPrivate
{
Q_DISABLE_COPY_MOVE(QRhiBufferDataPrivate)
QRhiBufferDataPrivate() { }
~QRhiBufferDataPrivate() { delete[] largeData; }
QRhiBufferDataPrivate() { } // don't value-initialize smallData
int ref = 1;
quint32 size = 0;
quint32 largeAlloc = 0;
char *largeData = nullptr;
QByteArray largeData;
static constexpr quint32 SMALL_DATA_SIZE = 1024;
char data[SMALL_DATA_SIZE];
char smallData[SMALL_DATA_SIZE];
};
// no detach-with-contents, no atomic refcount, no shrink
@ -363,7 +361,7 @@ public:
}
const char *constData() const
{
return d ? (d->size <= QRhiBufferDataPrivate::SMALL_DATA_SIZE ? d->data : d->largeData) : nullptr;
return d ? (d->size <= QRhiBufferDataPrivate::SMALL_DATA_SIZE ? d->smallData : d->largeData.constData()) : nullptr;
}
quint32 size() const
{
@ -371,7 +369,7 @@ public:
}
quint32 largeAlloc() const
{
return d ? d->largeAlloc : 0;
return d ? d->largeData.size() : 0;
}
void assign(const char *s, quint32 size)
{
@ -385,16 +383,28 @@ public:
}
d->size = size;
if (size <= QRhiBufferDataPrivate::SMALL_DATA_SIZE) {
memcpy(d->data, s, size);
memcpy(d->smallData, s, size);
} else {
if (d->largeAlloc < size) {
if (QRhiImplementation::rubLogEnabled)
qDebug("[rub] QRhiBufferData %p/%p new large data allocation %u -> %u", this, d, d->largeAlloc, size);
delete[] d->largeData;
d->largeAlloc = size;
d->largeData = new char[size];
}
memcpy(d->largeData, s, size);
if (QRhiImplementation::rubLogEnabled && largeAlloc() < size)
qDebug("[rub] QRhiBufferData %p/%p new large data allocation %u -> %u", this, d, largeAlloc(), size);
d->largeData.assign(QByteArrayView(s, size)); // keeps capacity
}
}
void assign(QByteArray data)
{
if (!d) {
d = new QRhiBufferDataPrivate;
} else if (d->ref != 1) {
if (QRhiImplementation::rubLogEnabled)
qDebug("[rub] QRhiBufferData %p/%p new backing due to no-copy detach, ref was %d", this, d, d->ref);
d->ref -= 1;
d = new QRhiBufferDataPrivate;
}
d->size = data.size();
if (d->size <= QRhiBufferDataPrivate::SMALL_DATA_SIZE) {
memcpy(d->smallData, data.constData(), data.size());
} else {
d->largeData = std::move(data);
}
}
private:
@ -435,6 +445,21 @@ public:
op->data.assign(reinterpret_cast<const char *>(data), effectiveSize);
}
static BufferOp dynamicUpdate(QRhiBuffer *buf, quint32 offset, QByteArray data)
{
BufferOp op = {};
changeToDynamicUpdate(&op, buf, offset, std::move(data));
return op;
}
static void changeToDynamicUpdate(BufferOp *op, QRhiBuffer *buf, quint32 offset, QByteArray data)
{
op->type = DynamicUpdate;
op->buf = buf;
op->offset = offset;
op->data.assign(std::move(data));
}
static BufferOp staticUpload(QRhiBuffer *buf, quint32 offset, quint32 size, const void *data)
{
BufferOp op = {};
@ -451,6 +476,21 @@ public:
op->data.assign(reinterpret_cast<const char *>(data), effectiveSize);
}
static BufferOp staticUpload(QRhiBuffer *buf, quint32 offset, QByteArray data)
{
BufferOp op = {};
changeToStaticUpload(&op, buf, offset, std::move(data));
return op;
}
static void changeToStaticUpload(BufferOp *op, QRhiBuffer *buf, quint32 offset, QByteArray data)
{
op->type = StaticUpload;
op->buf = buf;
op->offset = offset;
op->data.assign(std::move(data));
}
static BufferOp read(QRhiBuffer *buf, quint32 offset, quint32 size, QRhiReadbackResult *result)
{
BufferOp op = {};

7
tests/auto/gui/rhi/qrhi/tst_qrhi.cpp
View File

@ -6509,10 +6509,11 @@ void tst_QRhi::tessellationInterfaceBlocks()
u->updateDynamicBuffer(ubuf.data(), 0, 64, mvp.constData());
QScopedPointer<QRhiBuffer> buffer(
rhi->newBuffer(QRhiBuffer::Static, QRhiBuffer::UsageFlag::StorageBuffer, 1024));
rhi->newBuffer(QRhiBuffer::Static, QRhiBuffer::UsageFlag::StorageBuffer, 2048));
QVERIFY(buffer->create());
u->uploadStaticBuffer(buffer.data(), 0, 1024, QByteArray(1024, 0).constData());
// 2048 is a large buffer for RUB
u->uploadStaticBuffer(buffer.data(), QByteArray(2048, 0));
QScopedPointer<QRhiShaderResourceBindings> srb(rhi->newShaderResourceBindings());
srb->setBindings(
@ -6825,7 +6826,7 @@ void tst_QRhi::storageBuffer()
reinterpret_cast<float *>(&toGpuData.data()[toGpuMembers["_vec4"].offset])[2] = 9.0f;
reinterpret_cast<float *>(&toGpuData.data()[toGpuMembers["_vec4"].offset])[3] = 10.0f;
u->uploadStaticBuffer(toGpuBuffer.data(), 0, toGpuData.size(), toGpuData.constData());
u->uploadStaticBuffer(toGpuBuffer.data(), std::move(toGpuData));
u->uploadStaticBuffer(fromGpuBuffer.data(), 0, blocks["fromGpu"].knownSize, QByteArray(blocks["fromGpu"].knownSize, 0).constData());
QScopedPointer<QRhiShaderResourceBindings> srb(rhi->newShaderResourceBindings());