Permalink
Browse files

Fix occasional rendering glitch, under immediate mode interface.

We still support our old "immediate-mode"-style rendering interface,
where vertex data is embedded directly into the vsDisplayList, even
though modern graphic APIs don't support that.  We just push the data
into a temporary VBO and render from it as the data comes to us.

The issue is that we're following a naive streaming strategy for pushing
that data to OpenGL;  we just keep tacking on new data until the buffer
fills up, then we orphan the buffer and keep going.  But if that "run
out of space" thing happens in the middle of setting up a draw, you wind
up orphaning a buffer which contained data needed for the draw, before
you actually kicked off a draw that was using the data.  This would lead
to draws with some invalid vertex attributes.

This commit changes things so that we wait until just before issuing a
draw call, and then bind all of our vertex attributes all at once,
instead of doing it precisely when the display list told us to do it.
This means that we can test in advance that *all* our vertex data will
fit into the shared VBO at the same time, instead of running out of
space partway through and getting a corrupt draw.
  • Loading branch information...
Trevor Powell
Trevor Powell committed Mar 3, 2018
1 parent 0ce678a commit 3feced5d3a5ce1c76a1a47b6225671fb87337e56
Showing with 93 additions and 7 deletions.
  1. +30 −1 VS/Graphics/VS_RenderBuffer.cpp
  2. +1 −0 VS/Graphics/VS_RenderBuffer.h
  3. +62 −6 VS/Graphics/VS_Renderer_OpenGL3.cpp
@@ -1074,6 +1074,7 @@ static int g_vboCursor = VBO_SIZE;
void
vsRenderBuffer::BindArrayToAttribute( void* buffer, size_t bufferSize, int attribute, int elementCount )
{
vsAssert(bufferSize < VBO_SIZE, "Tried to bind too large an array for VBO_SIZE?");
if ( g_vbo == 0xffffffff )
{
glGenBuffers(1, &g_vbo);
@@ -1083,7 +1084,10 @@ vsRenderBuffer::BindArrayToAttribute( void* buffer, size_t bufferSize, int attri
if ( g_vboCursor + bufferSize >= VBO_SIZE )
{
glBufferData(GL_ARRAY_BUFFER, VBO_SIZE, NULL, GL_STREAM_DRAW);
// This shouldn't happen any more; we should always catch this in the
// "EnsureSpaceForVertexColorTexelNormal()" function below. But leave this
// here just for safety for the moment.
glBufferData(GL_ARRAY_BUFFER, VBO_SIZE, NULL, GL_DYNAMIC_DRAW);
g_vboCursor = 0;
}
glBufferSubData(GL_ARRAY_BUFFER, g_vboCursor, bufferSize, buffer);
@@ -1094,6 +1098,31 @@ vsRenderBuffer::BindArrayToAttribute( void* buffer, size_t bufferSize, int attri
g_vboCursor += bufferSize;
}
void
vsRenderBuffer::EnsureSpaceForVertexColorTexelNormal( int vertexCount, int colorCount, int texelCount, int normalCount )
{
int bufferSize = vertexCount * sizeof(vsVector3D) +
colorCount * sizeof(vsColor) +
texelCount * sizeof(vsVector2D) +
normalCount * sizeof(vsVector2D);
if ( g_vboCursor + bufferSize >= VBO_SIZE )
{
vsAssert(bufferSize < VBO_SIZE, "Tried to bind too large an array for VBO_SIZE?");
if ( g_vbo == 0xffffffff )
{
glGenBuffers(1, &g_vbo);
}
glBindBuffer(GL_ARRAY_BUFFER, g_vbo);
// orphan the buffer and start on the new one.
glBufferData(GL_ARRAY_BUFFER, VBO_SIZE, NULL, GL_DYNAMIC_DRAW);
g_vboCursor = 0;
glBindBuffer(GL_ARRAY_BUFFER, 0);
}
}
void
vsRenderBuffer::BindVertexArray( vsRendererState *state, void* buffer, int vertexCount )
{
@@ -234,6 +234,7 @@ class vsRenderBuffer
void Bind( vsRendererState *state ); // for non-custom types
void Unbind( vsRendererState *state ); // for non-custom types
static void EnsureSpaceForVertexColorTexelNormal( int vertexCount, int colorCount, int texelCount, int normalCount );
static void BindArrayToAttribute( void* buffer, size_t bufferSize, int attribute, int elementCount );
static void BindVertexArray( vsRendererState *state, void* buffer, int vertexCount );
static void BindColorArray( vsRendererState *state, void* buffer, int vertexCount );
@@ -691,6 +691,42 @@ vsRenderer_OpenGL3::RenderDisplayList( vsDisplayList *list )
void
vsRenderer_OpenGL3::FlushRenderState()
{
// For these immediate-mode style "arrays embedded directly in the display list"
// situations, we need to make sure that we have enough space to push all the
// data directly; that we won't run out partway through. This means that we
// couldn't bind the arrays immediately as they came in, but instead needed to
// hold on to them until now, right before we draw. So let's make sure we
// have space for all the data, then bind it all at once!
if ( m_currentColorArray || m_currentNormalArray || m_currentTexelArray || m_currentVertexArray )
{
vsRenderBuffer::EnsureSpaceForVertexColorTexelNormal(
m_currentVertexArrayCount,
m_currentColorArrayCount,
m_currentTexelArrayCount,
m_currentNormalArrayCount
);
}
if ( m_currentColorArray )
{
vsRenderBuffer::BindColorArray( &m_state, m_currentColorArray, m_currentColorArrayCount );
m_state.SetBool( vsRendererState::ClientBool_ColorArray, true );
}
if ( m_currentNormalArray )
{
vsRenderBuffer::BindNormalArray( &m_state, m_currentNormalArray, m_currentNormalArrayCount );
m_state.SetBool( vsRendererState::ClientBool_NormalArray, true );
}
if ( m_currentTexelArray )
{
vsRenderBuffer::BindTexelArray( &m_state, m_currentTexelArray, m_currentTexelArrayCount );
m_state.SetBool( vsRendererState::ClientBool_TextureCoordinateArray, true );
}
if ( m_currentVertexArray )
{
vsRenderBuffer::BindVertexArray( &m_state, m_currentVertexArray, m_currentVertexArrayCount );
m_state.SetBool( vsRendererState::ClientBool_VertexArray, true );
}
// CheckGLError("EnsureSpaceForVertex");
// CheckGLError("PreFlush");
static vsMaterial *s_previousMaterial = NULL;
static vsShaderValues *s_previousShaderValues = NULL;
@@ -755,6 +791,7 @@ vsRenderer_OpenGL3::FlushRenderState()
vsAssert(0, "Trying to flush render state with no shader set?");
glUseProgram( 0 );
}
}
void
@@ -985,8 +1022,6 @@ vsRenderer_OpenGL3::RawRenderDisplayList( vsDisplayList *list )
m_currentVertexArray = (vsVector3D*)op->data.p;
m_currentVertexArrayCount = op->data.i;
m_currentVertexBuffer = NULL;
vsRenderBuffer::BindVertexArray( &m_state, op->data.p, op->data.i );
m_state.SetBool( vsRendererState::ClientBool_VertexArray, true );
break;
}
case vsDisplayList::OpCode_VertexBuffer:
@@ -999,14 +1034,16 @@ vsRenderer_OpenGL3::RawRenderDisplayList( vsDisplayList *list )
}
case vsDisplayList::OpCode_NormalArray:
{
vsRenderBuffer::BindNormalArray( &m_state, op->data.p, op->data.i );
m_state.SetBool( vsRendererState::ClientBool_NormalArray, true );
m_currentNormalArray = (vsVector3D*)op->data.p;
m_currentNormalArrayCount = op->data.i;
break;
}
case vsDisplayList::OpCode_NormalBuffer:
{
m_currentNormalBuffer = (vsRenderBuffer *)op->data.p;
m_currentNormalBuffer->BindNormalBuffer( &m_state );
m_currentNormalArray = NULL;
m_currentNormalArrayCount = 0;
m_state.SetBool( vsRendererState::ClientBool_NormalArray, true );
break;
}
@@ -1028,13 +1065,17 @@ vsRenderer_OpenGL3::RawRenderDisplayList( vsDisplayList *list )
}
case vsDisplayList::OpCode_TexelArray:
{
m_currentTexelArray = (vsVector2D*)op->data.p;
m_currentTexelArrayCount = op->data.i;
vsRenderBuffer::BindTexelArray( &m_state, op->data.p, op->data.i );
m_state.SetBool( vsRendererState::ClientBool_TextureCoordinateArray, true );
break;
}
case vsDisplayList::OpCode_TexelBuffer:
{
m_currentTexelBuffer = (vsRenderBuffer *)op->data.p;
m_currentTexelArray = NULL;
m_currentTexelArrayCount = 0;
m_currentTexelBuffer->BindTexelBuffer( &m_state );
break;
}
@@ -1045,14 +1086,16 @@ vsRenderer_OpenGL3::RawRenderDisplayList( vsDisplayList *list )
}
case vsDisplayList::OpCode_ColorArray:
{
vsRenderBuffer::BindColorArray( &m_state, op->data.p, op->data.i );
m_state.SetBool( vsRendererState::ClientBool_ColorArray, true );
m_currentColorArray = (vsColor*)op->data.p;
m_currentColorArrayCount = op->data.i;
break;
}
case vsDisplayList::OpCode_ColorBuffer:
{
m_currentColorBuffer = (vsRenderBuffer *)op->data.p;
m_currentColorBuffer->BindColorBuffer( &m_state );
m_currentColorArray = 0;
m_currentColorArrayCount = 0;
break;
}
case vsDisplayList::OpCode_ClearColorArray:
@@ -1093,6 +1136,19 @@ vsRenderer_OpenGL3::RawRenderDisplayList( vsDisplayList *list )
case vsDisplayList::OpCode_BindBuffer:
{
PROFILE_GL("BindBuffer");
m_currentColorArray = NULL;
m_currentColorBuffer = NULL;
m_currentColorArrayCount = 0;
m_currentTexelBuffer = NULL;
m_currentTexelArray = NULL;
m_currentTexelArrayCount = 0;
m_currentNormalBuffer = NULL;
m_currentNormalArray = NULL;
m_currentNormalArrayCount = 0;
m_currentVertexBuffer = NULL;
m_currentVertexArray = NULL;
m_currentVertexArrayCount = 0;
vsRenderBuffer *buffer = (vsRenderBuffer *)op->data.p;
buffer->Bind( &m_state );
break;

0 comments on commit 3feced5

Please sign in to comment.