Potential unwanted/undesirable behavior in Render2D (particularly in the c++ game engine series)

[tekglitch]

(NOTE: it is possible this has been revised/fixed in later updates. as the video this bug is pertaining to is about a year old.)

You pass s_Data.QuadIndexCount to drawIndexed() in Renderer2d::flush(), but inside drawIndexed() you check if (indexcount == 0) and use IndexBuffer()->getCount() if it is 0. Meaning if you don't draw any quads during a frame(indexcount==0) drawIndexed will quarry the stored index buffer values and pull that index count(IndexBuffer()->getCount())(which is still valid in memory) as well as the previous stored vertex data as it's already been buffered to the gpu, causing the last stored "frame" to be re-rendered.

void Renderer::drawIndexed(const std::shared_ptr<VertexArray>& vArray, uint32_t icount)
{
	uint32_t count = (icount == 0) ? vArray->getIndexBuffer()->getCount() : icount;
	glDrawElements(GL_TRIANGLES, icount, GL_UNSIGNED_INT, nullptr);
}

To Reproduce

Draw some quads to the screen. Short time after, stop drawing the quads. If you have your vertex buffer stored the renderer (upon getting a draw index count of 0) the indexbuffer will return it's count instead (max quads * 4 i beleive) if the vertex data is still present in memory the default "index count" will cause that vertex data to be redrawn. this will lead to anything from random flickering to entire scenes being redrawn.

Expected behavior

It is fairly obviously if the renderer sends 0 to drawIndexed it should just not draw or draw 0. vs sending it the default maxCount stored in the index buffer.(which is always max* 6)

Screenshots

If applicable, add extra screenshots to help explain your problem.

Operating system: (please complete the following information)

win64 - 10 home

Additional context

There is a good chance you fixed this in one of your videos.

[tekglitch]

Alternatively, you can make sure any stored vertex buffer data is cleared at the beginning of each frame. Note this seems to derive from using glBufferSubData(). opengl apparently keeps the buffered data persistently across each frame (maybe relating to batch rendering?) either way the previous frame's vertex buffer is still in memory and being read by the gpu even without any direct draw calls to the renderer

[drsnuggles8]

Have you checked if this code is even still in the current version of Hazel?

[VagueLobster]

It's been changed since then to now use: void OpenGLRendererAPI::DrawIndexed(const Ref<VertexArray>& vertexArray, uint32_t indexCount) { vertexArray->Bind(); uint32_t count = indexCount ? indexCount : vertexArray->GetIndexBuffer()->GetCount(); glDrawElements(GL_TRIANGLES, count, GL_UNSIGNED_INT, nullptr); }

So don't worry about the bug, @TheCherno has refactored the Renderer at a later point than what this video you mention is from 👍 Stupid code formatting here on Github 😠 Also, after the C# Scripting implementation is done (it's what he's currently working on in the game-engine-series) He plans to change the Renderer again, but instead of using OpenGL, he will use Vulkan instead and remove OpenGL👍

[tekglitch]

Thanks for the info Vague! Much appreciated. I had assumed this was most likely fixed... but in that small off chance, it was overlooked I figured I'd bring it up.

[lvotusk]

I just got caught out by this too, so just a bit of further information...

This is a throwback as to when Batch Rendering was implemented... the old API call used the index count from the IndexBuffer in the Vertex Array... to utilise the new batch rendering system DrawIndexed method was altered adding indexCount parameter with default = 0 The intention was to allow existing DrawIndexed( ... ) calls with the default indexCount 0 value indicating to use the index count from the vertex array's IndexBuffer. However, any DrawIndexed( ... ) calls with a genuine indexCount of 0 passed to the method could cause erroneous / unintended behaviour.

The Renderer2D::Flush() method now includes a guard to check for a 0 indexCount before calling OpenGLRendererAPI::DrawIndexed( ... )