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

Fix: Fix Uniform buffer doing too many iterations when updating renderer uniforms #3372

Merged

Conversation

LittleCodingFox
Copy link
Contributor

When rendering 1275 drawcalls with heavy uniform use without discarding textures and uniforms, I was having terrible performance, worse than when discarding.

While investigating, I realized that when BGFX calls rendererUpdateUniforms, it always had _begin set to 0.

I looked at how EncoderImpl::dispatch always sets the m_uniformBegin to m_uniformEnd after each item is added, so I did the same for EncoderImpl::submit, and the issue is fixed! I actually have much better performance than when using discards, which is to be expected!

As requested, I tested the examples and they seem to work fine, so I believe there should be no side effects!

Copy link
Owner

@bkaradzic bkaradzic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modify existing example from repo to demonstrate the problem you're having.

@LittleCodingFox
Copy link
Contributor Author

I modified the hextile sample like so:

  • Make it render the terrain 10000 times with BGFX_DISCARD_NONE

Tested in debug to prevent optimizations

Before my fix:

BGFX_Before

After my fix:

BGFX_After

@bkaradzic
Copy link
Owner

I modified the hextile sample like so:

Do diff or paste code here so that I can use exactly what you're using, instead creating something that might not match what you have.

@LittleCodingFox
Copy link
Contributor Author

Replace line 286 of hextile.cpp with:

				for (int i = 0; i < 10000; i++)
				{
					bgfx::submit(0, m_hextileProgram, 0, BGFX_DISCARD_NONE);
				}

@LittleCodingFox
Copy link
Contributor Author

LittleCodingFox commented Nov 11, 2024

Also, these changes are essentially a 1:1 to how CS already did things:

(bgfx.cpp:1359)

		uint64_t key = m_key.encodeCompute();
		m_frame->m_sortKeys[renderItemIdx]   = key;
		m_frame->m_sortValues[renderItemIdx] = RenderItemCount(renderItemIdx);

		m_compute.m_uniformIdx   = m_uniformIdx;
		m_compute.m_uniformBegin = m_uniformBegin;
		m_compute.m_uniformEnd   = m_uniformEnd;
		m_frame->m_renderItem[renderItemIdx].compute = m_compute;
		m_frame->m_renderItemBind[renderItemIdx]     = m_bind;

		m_compute.clear(_flags);
		m_bind.clear(_flags);
		m_uniformBegin = m_uniformEnd;

@bkaradzic bkaradzic merged commit 4bc6529 into bkaradzic:master Nov 11, 2024
11 checks passed
@LittleCodingFox LittleCodingFox deleted the feature/uniform-discard-state-fix branch November 11, 2024 08:44
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

Successfully merging this pull request may close these issues.

2 participants