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

rhi: gl: Reset tracked state upon a buffer update or readback

Browse Source 
...encountered in the command list.

Move all, previously local, tracking variables into a struct. This
allows creating helper functions to reduce error-prone repetition in the
executeCommandBuffer() function body.

The only real change in the patch is in the handling of
Command::BufferSubData and Command::GetBufferSubData: here, instead of
calling glBindBuffer directly, use a helper function that also resets
the relevant state tracking variables. A subsequent
Command::BindVertexBuffer or BindIndexBuffer will therefore correctly
rebind the appropriate buffers.

This is particularly relevant with certain command stream patterns
exercised by some Qt Quick 3D scenes:

- A View3D renders a mesh,
- another View3D has some 2D Qt Quick content, as well as a model with
  the same mesh.

When both View3Ds use the default Offscreen render mode, the resulting
command list consists of segments along the lines of:

1. prepare resources for first View3D

2. render content for first View3D - this binds the vertex and index
   buffers for the mesh (state is tracked; all 1-4 steps are within
   the same command list, processed by a single call to
   executeCommandBuffer())

3. prepare the content for the "inline" 2D Qt Quick scene - this may
   update vertex and index buffers, that may lead to adding
   BufferSubData commands to the list (tracked state (last
   vertex/index buffer) may need invalidation/updating - and that's
   where our problem lies)

4. the second View3Ds 3D content is rendered: a model with the same
   mesh as the last (Quick)3D draw call, so same vertex and index
   buffers. If #3 did not invalidate and/or update the tracked state,
   the glBindBuffer calls are (incorrectly) skipped.

Fixes: QTBUG-89780
Change-Id: Icc933252f3993b8727d192e7ba4aa0f842bab51e
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
(cherry picked from commit 5f8efb259774040303f37d00b6307afd22857af2)
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Laszlo Agocs <laszlo.agocs@qt.io>
This commit is contained in:
Laszlo Agocs 2021-01-05 15:48:32 +01:00
parent 5aa3cf7b9a
commit 1d7fc87b89
1 changed files with 51 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
src/gui/rhi/qrhigles2.cpp
View File

@ -2120,9 +2120,8 @@ void QRhiGles2::trackedRegisterTexture(QRhiPassResourceTracker *passResTracker,
u.access = toGlAccess(access); u.access = toGlAccess(access);
} }
void QRhiGles2::executeCommandBuffer(QRhiCommandBuffer *cb) struct CommandBufferExecTrackedState
{ {
QGles2CommandBuffer *cbD = QRHI_RES(QGles2CommandBuffer, cb);
GLenum indexType = GL_UNSIGNED_SHORT; GLenum indexType = GL_UNSIGNED_SHORT;
quint32 indexStride = sizeof(quint16); quint32 indexStride = sizeof(quint16);
quint32 indexOffset = 0; quint32 indexOffset = 0;
@ -2135,8 +2134,27 @@ void QRhiGles2::executeCommandBuffer(QRhiCommandBuffer *cb)
int binding = 0; int binding = 0;
} lastBindVertexBuffer; } lastBindVertexBuffer;
static const int TRACKED_ATTRIB_COUNT = 16; static const int TRACKED_ATTRIB_COUNT = 16;
bool enabledAttribArrays[TRACKED_ATTRIB_COUNT]; bool enabledAttribArrays[TRACKED_ATTRIB_COUNT] = {};
memset(enabledAttribArrays, 0, sizeof(enabledAttribArrays)); };
// Helper that must be used in executeCommandBuffer() whenever changing the
// ARRAY or ELEMENT_ARRAY buffer binding outside of Command::BindVertexBuffer
// and Command::BindIndexBuffer.
static inline void bindVertexIndexBufferWithStateReset(CommandBufferExecTrackedState *state,
QOpenGLExtensions *f,
GLenum target,
GLuint buffer)
{
state->currentArrayBuffer = 0;
state->currentElementArrayBuffer = 0;
state->lastBindVertexBuffer.buffer = 0;
f->glBindBuffer(target, buffer);
}
void QRhiGles2::executeCommandBuffer(QRhiCommandBuffer *cb)
{
CommandBufferExecTrackedState state;
QGles2CommandBuffer *cbD = QRHI_RES(QGles2CommandBuffer, cb);
for (auto it = cbD->commands.cbegin(), end = cbD->commands.cend(); it != end; ++it) { for (auto it = cbD->commands.cbegin(), end = cbD->commands.cend(); it != end; ++it) {
const QGles2CommandBuffer::Command &cmd(*it); const QGles2CommandBuffer::Command &cmd(*it);
@ -2183,25 +2201,25 @@ void QRhiGles2::executeCommandBuffer(QRhiCommandBuffer *cb)
{ {
QGles2GraphicsPipeline *psD = QRHI_RES(QGles2GraphicsPipeline, cmd.args.bindVertexBuffer.ps); QGles2GraphicsPipeline *psD = QRHI_RES(QGles2GraphicsPipeline, cmd.args.bindVertexBuffer.ps);
if (psD) { if (psD) {
if (lastBindVertexBuffer.ps == psD if (state.lastBindVertexBuffer.ps == psD
&& lastBindVertexBuffer.buffer == cmd.args.bindVertexBuffer.buffer && state.lastBindVertexBuffer.buffer == cmd.args.bindVertexBuffer.buffer
&& lastBindVertexBuffer.offset == cmd.args.bindVertexBuffer.offset && state.lastBindVertexBuffer.offset == cmd.args.bindVertexBuffer.offset
&& lastBindVertexBuffer.binding == cmd.args.bindVertexBuffer.binding) && state.lastBindVertexBuffer.binding == cmd.args.bindVertexBuffer.binding)
{ {
// The pipeline and so the vertex input layout is // The pipeline and so the vertex input layout is
// immutable, no point in issuing the exact same set of // immutable, no point in issuing the exact same set of
// glVertexAttribPointer again and again for the same buffer. // glVertexAttribPointer again and again for the same buffer.
break; break;
} }
lastBindVertexBuffer.ps = psD; state.lastBindVertexBuffer.ps = psD;
lastBindVertexBuffer.buffer = cmd.args.bindVertexBuffer.buffer; state.lastBindVertexBuffer.buffer = cmd.args.bindVertexBuffer.buffer;
lastBindVertexBuffer.offset = cmd.args.bindVertexBuffer.offset; state.lastBindVertexBuffer.offset = cmd.args.bindVertexBuffer.offset;
lastBindVertexBuffer.binding = cmd.args.bindVertexBuffer.binding; state.lastBindVertexBuffer.binding = cmd.args.bindVertexBuffer.binding;
if (cmd.args.bindVertexBuffer.buffer != currentArrayBuffer) { if (cmd.args.bindVertexBuffer.buffer != state.currentArrayBuffer) {
currentArrayBuffer = cmd.args.bindVertexBuffer.buffer; state.currentArrayBuffer = cmd.args.bindVertexBuffer.buffer;
// we do not support more than one vertex buffer // we do not support more than one vertex buffer
f->glBindBuffer(GL_ARRAY_BUFFER, currentArrayBuffer); f->glBindBuffer(GL_ARRAY_BUFFER, state.currentArrayBuffer);
} }
for (auto it = psD->m_vertexInputLayout.cbeginAttributes(), itEnd = psD->m_vertexInputLayout.cendAttributes(); for (auto it = psD->m_vertexInputLayout.cbeginAttributes(), itEnd = psD->m_vertexInputLayout.cendAttributes();
it != itEnd; ++it) it != itEnd; ++it)
@ -2292,16 +2310,16 @@ void QRhiGles2::executeCommandBuffer(QRhiCommandBuffer *cb)
} else { } else {
qWarning("Current RHI backend does not support IntAttributes. Check supported features."); qWarning("Current RHI backend does not support IntAttributes. Check supported features.");
// This is a trick to disable this attribute // This is a trick to disable this attribute
if (locationIdx < TRACKED_ATTRIB_COUNT) if (locationIdx < CommandBufferExecTrackedState::TRACKED_ATTRIB_COUNT)
enabledAttribArrays[locationIdx] = true; state.enabledAttribArrays[locationIdx] = true;
} }
} else { } else {
f->glVertexAttribPointer(GLuint(locationIdx), size, type, normalize, stride, f->glVertexAttribPointer(GLuint(locationIdx), size, type, normalize, stride,
reinterpret_cast<const GLvoid *>(quintptr(ofs))); reinterpret_cast<const GLvoid *>(quintptr(ofs)));
} }
if (locationIdx >= TRACKED_ATTRIB_COUNT || !enabledAttribArrays[locationIdx]) { if (locationIdx >= CommandBufferExecTrackedState::TRACKED_ATTRIB_COUNT || !state.enabledAttribArrays[locationIdx]) {
if (locationIdx < TRACKED_ATTRIB_COUNT) if (locationIdx < CommandBufferExecTrackedState::TRACKED_ATTRIB_COUNT)
enabledAttribArrays[locationIdx] = true; state.enabledAttribArrays[locationIdx] = true;
f->glEnableVertexAttribArray(GLuint(locationIdx)); f->glEnableVertexAttribArray(GLuint(locationIdx));
} }
if (inputBinding->classification() == QRhiVertexInputBinding::PerInstance && caps.instancing) if (inputBinding->classification() == QRhiVertexInputBinding::PerInstance && caps.instancing)
@ -2313,12 +2331,12 @@ void QRhiGles2::executeCommandBuffer(QRhiCommandBuffer *cb)
} }
break; break;
case QGles2CommandBuffer::Command::BindIndexBuffer: case QGles2CommandBuffer::Command::BindIndexBuffer:
indexType = cmd.args.bindIndexBuffer.type; state.indexType = cmd.args.bindIndexBuffer.type;
indexStride = indexType == GL_UNSIGNED_SHORT ? sizeof(quint16) : sizeof(quint32); state.indexStride = state.indexType == GL_UNSIGNED_SHORT ? sizeof(quint16) : sizeof(quint32);
indexOffset = cmd.args.bindIndexBuffer.offset; state.indexOffset = cmd.args.bindIndexBuffer.offset;
if (currentElementArrayBuffer != cmd.args.bindIndexBuffer.buffer) { if (state.currentElementArrayBuffer != cmd.args.bindIndexBuffer.buffer) {
currentElementArrayBuffer = cmd.args.bindIndexBuffer.buffer; state.currentElementArrayBuffer = cmd.args.bindIndexBuffer.buffer;
f->glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, currentElementArrayBuffer); f->glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, state.currentElementArrayBuffer);
} }
break; break;
case QGles2CommandBuffer::Command::Draw: case QGles2CommandBuffer::Command::Draw:
@ -2341,32 +2359,32 @@ void QRhiGles2::executeCommandBuffer(QRhiCommandBuffer *cb)
QGles2GraphicsPipeline *psD = QRHI_RES(QGles2GraphicsPipeline, cmd.args.drawIndexed.ps); QGles2GraphicsPipeline *psD = QRHI_RES(QGles2GraphicsPipeline, cmd.args.drawIndexed.ps);
if (psD) { if (psD) {
const GLvoid *ofs = reinterpret_cast<const GLvoid *>( const GLvoid *ofs = reinterpret_cast<const GLvoid *>(
quintptr(cmd.args.drawIndexed.firstIndex * indexStride + indexOffset)); quintptr(cmd.args.drawIndexed.firstIndex * state.indexStride + state.indexOffset));
if (cmd.args.drawIndexed.instanceCount == 1 || !caps.instancing) { if (cmd.args.drawIndexed.instanceCount == 1 || !caps.instancing) {
if (cmd.args.drawIndexed.baseVertex != 0 && caps.baseVertex) { if (cmd.args.drawIndexed.baseVertex != 0 && caps.baseVertex) {
f->glDrawElementsBaseVertex(psD->drawMode, f->glDrawElementsBaseVertex(psD->drawMode,
GLsizei(cmd.args.drawIndexed.indexCount), GLsizei(cmd.args.drawIndexed.indexCount),
indexType, state.indexType,
ofs, ofs,
cmd.args.drawIndexed.baseVertex); cmd.args.drawIndexed.baseVertex);
} else { } else {
f->glDrawElements(psD->drawMode, f->glDrawElements(psD->drawMode,
GLsizei(cmd.args.drawIndexed.indexCount), GLsizei(cmd.args.drawIndexed.indexCount),
indexType, state.indexType,
ofs); ofs);
} }
} else { } else {
if (cmd.args.drawIndexed.baseVertex != 0 && caps.baseVertex) { if (cmd.args.drawIndexed.baseVertex != 0 && caps.baseVertex) {
f->glDrawElementsInstancedBaseVertex(psD->drawMode, f->glDrawElementsInstancedBaseVertex(psD->drawMode,
GLsizei(cmd.args.drawIndexed.indexCount), GLsizei(cmd.args.drawIndexed.indexCount),
indexType, state.indexType,
ofs, ofs,
GLsizei(cmd.args.drawIndexed.instanceCount), GLsizei(cmd.args.drawIndexed.instanceCount),
cmd.args.drawIndexed.baseVertex); cmd.args.drawIndexed.baseVertex);
} else { } else {
f->glDrawElementsInstanced(psD->drawMode, f->glDrawElementsInstanced(psD->drawMode,
GLsizei(cmd.args.drawIndexed.indexCount), GLsizei(cmd.args.drawIndexed.indexCount),
indexType, state.indexType,
ofs, ofs,
GLsizei(cmd.args.drawIndexed.instanceCount)); GLsizei(cmd.args.drawIndexed.instanceCount));
} }
@ -2423,14 +2441,14 @@ void QRhiGles2::executeCommandBuffer(QRhiCommandBuffer *cb)
cbD->graphicsPassState.reset(); // altered depth/color write, invalidate in order to avoid confusing the state tracking cbD->graphicsPassState.reset(); // altered depth/color write, invalidate in order to avoid confusing the state tracking
break; break;
case QGles2CommandBuffer::Command::BufferSubData: case QGles2CommandBuffer::Command::BufferSubData:
f->glBindBuffer(cmd.args.bufferSubData.target, cmd.args.bufferSubData.buffer); bindVertexIndexBufferWithStateReset(&state, f, cmd.args.bufferSubData.target, cmd.args.bufferSubData.buffer);
f->glBufferSubData(cmd.args.bufferSubData.target, cmd.args.bufferSubData.offset, cmd.args.bufferSubData.size, f->glBufferSubData(cmd.args.bufferSubData.target, cmd.args.bufferSubData.offset, cmd.args.bufferSubData.size,
cmd.args.bufferSubData.data); cmd.args.bufferSubData.data);
break; break;
case QGles2CommandBuffer::Command::GetBufferSubData: case QGles2CommandBuffer::Command::GetBufferSubData:
{ {
QRhiBufferReadbackResult *result = cmd.args.getBufferSubData.result; QRhiBufferReadbackResult *result = cmd.args.getBufferSubData.result;
f->glBindBuffer(cmd.args.getBufferSubData.target, cmd.args.getBufferSubData.buffer); bindVertexIndexBufferWithStateReset(&state, f, cmd.args.getBufferSubData.target, cmd.args.getBufferSubData.buffer);
if (caps.gles) { if (caps.gles) {
if (caps.properMapBuffer) { if (caps.properMapBuffer) {
void *p = f->glMapBufferRange(cmd.args.getBufferSubData.target, void *p = f->glMapBufferRange(cmd.args.getBufferSubData.target,