Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nanovg with BGFX Fill Bug when many nvgFill calls are made #1887

Closed
kestrelm opened this issue Sep 15, 2019 · 4 comments
Closed

Nanovg with BGFX Fill Bug when many nvgFill calls are made #1887

kestrelm opened this issue Sep 15, 2019 · 4 comments

Comments

@kestrelm
Copy link

Hello,
This is in reference to this bug: #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
Copy link
Author

kestrelm commented Sep 15, 2019

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
Copy link
Contributor

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

@mcourteaux
Copy link
Contributor

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

@mcourteaux
Copy link
Contributor

Fixed in #3207.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants