Nanovg with BGFX Fill Bug when many nvgFill calls are made

[kestrelm]

Hello, This is in reference to this bug: https://github.com/bkaradzic/bgfx/issues/1671

I took a look at the code and am wondering if this can be a potential problem:

static void fan(uint32_t _start, uint32_t _count)
{
		uint32_t numTris = _count-2;
		bgfx::TransientIndexBuffer tib;
		bgfx::allocTransientIndexBuffer(&tib, numTris*3);
		uint16_t* data = (uint16_t*)tib.data;
		for (uint32_t ii = 0; ii < numTris; ++ii)
		{
			data[ii*3+0] = _start; // What if _start is > unsigned 16 bit?
			data[ii*3+1] = _start + ii + 1;
			data[ii*3+2] = _start + ii + 2;
		}

		bgfx::setIndexBuffer(&tib);
}

The docs state that the transient index buffers only support 16-bit index buffers. If we are drawing a large number of objects with nanogvg, could it be that we can possibly exceed the 16-bit buffer limitation? Any way to get around this possible problem?

Thanks

[kestrelm]

Hello, Yes I think that could be the issue. I made a small change to test out the theory in glnvg__convexFill() and it seems to address the problem:

static void glnvg__convexFill(struct GLNVGcontext* gl, struct GLNVGcall* call)
	{
		struct GLNVGpath* paths = &gl->paths[call->pathOffset];
		int i, npaths = call->pathCount;

		nvgRenderSetUniforms(gl, call->uniformOffset, call->image);

		for (i = 0; i < npaths; i++)
		{
			if (paths[i].fillCount == 0) continue;
			bgfx::setState(gl->state);
			const uint32_t MAX_TRANSIENT_INDEX_SIZE = 65536;
			if (paths[i].fillOffset >= MAX_TRANSIENT_INDEX_SIZE)
			{
				// Because we have exceeded the 16-bit transient index buffer limitation in BGFX, we need to
				// form a separate transient vertex buffer for each fan separately. This is an issue with BGFX which
				// needs to be addressed
				uint32_t numTris = paths[i].fillCount - 2;
				bgfx::TransientVertexBuffer tmpVBuffer;
				bgfx::allocTransientVertexBuffer(&tmpVBuffer, numTris * 3, s_nvgDecl);
				for (uint32_t ii = 0; ii < numTris; ++ii)
				{
					for (uint32_t jj = 0; jj < 3; jj++)
					{
						uint32_t vIdx = (ii * 3 + jj);
						uint32_t tIdx = paths[i].fillOffset + vIdx;
						bx::memCopy(
							tmpVBuffer.data + (vIdx * sizeof(struct NVGvertex)),
							gl->verts + tIdx,
							sizeof(struct NVGvertex));
					}					
				}
				bgfx::setVertexBuffer(0, &tmpVBuffer);
				bgfx::setTexture(0, gl->s_tex, gl->th);

				bgfx::TransientIndexBuffer tib;
				bgfx::allocTransientIndexBuffer(&tib, numTris * 3);
				uint16_t* data = (uint16_t*)tib.data;
				for (uint32_t ii = 0; ii < numTris; ++ii)
				{
					data[ii * 3 + 0] = 0;
					data[ii * 3 + 1] = ii + 1;
					data[ii * 3 + 2] = ii + 2;
				}

				bgfx::setIndexBuffer(&tib);
				bgfx::submit(gl->viewId, gl->prog);
			}
			else {
				bgfx::setVertexBuffer(0, &gl->tvb);
				bgfx::setTexture(0, gl->s_tex, gl->th);
				fan(paths[i].fillOffset, paths[i].fillCount);
				bgfx::submit(gl->viewId, gl->prog);
			}
		}

		if (gl->edgeAntiAlias)
		{
			// Draw fringes
			for (i = 0; i < npaths; i++)
			{
				bgfx::setState(gl->state
					| BGFX_STATE_PT_TRISTRIP
					);
				bgfx::setVertexBuffer(0, &gl->tvb, paths[i].strokeOffset, paths[i].strokeCount);
				bgfx::setTexture(0, gl->s_tex, gl->th);
				bgfx::submit(gl->viewId, gl->prog);
			}
		}
	}

I am not familiar with the entire code base so I will leave it up to you to refactor ( or rewrite something ) similar for this problem. But it does look like due to the limitation of the 16-bit transient index buffer, this issue occurs. Granted, this is not the most efficient solution but it does serve to hopefully guide you to a cleaner solution.

Thanks

[mcourteaux]

This fixes the issue for like 95% of the cases. Sometimes, there is still a glitch. Will investigate this sometime soon.

[mcourteaux]

I'm assuming the same overflow can happen when rendering strokes instead?

[mcourteaux]

Fixed in #3207.