Skip to content

Implement PushBuffer #1684

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

VReaperV
Copy link
Contributor

@VReaperV VReaperV commented Jun 23, 2025

Requires #1683

Add PushBuffer, update uniforms to have an updateType. The CONST and FRAME uniforms are written into a uniform buffer after the map is loaded/at the start of a frame. This allows skipping some of the glUniform*() calls to decrease the amount of overhead.

This largely re-uses the existing functionality added for the material system.

This will also be needed for #1587

@VReaperV VReaperV added T-Improvement Improvement for an existing feature A-Renderer T-Feature-Request Proposed new feature labels Jun 23, 2025
@VReaperV VReaperV force-pushed the push-buffer-ubo branch 3 times, most recently from 1e2423c to 1bd0d74 Compare June 30, 2025 15:26
@slipher
Copy link
Member

slipher commented Jul 13, 2025

This can be rebased now.

@VReaperV
Copy link
Contributor Author

This can be rebased now.

Done.

@slipher
Copy link
Member

slipher commented Jul 24, 2025

Some issues found with testing:

  1. Fog is not rendered if material system is enabled.
  2. In one scene from my test script the dynamic light from the Painsaw is not rendered. I can't observe this with manual testing so I'd have to investigate more to provide a reliable repro.

@slipher
Copy link
Member

slipher commented Jul 24, 2025

2. In one scene from my test script the dynamic light from the Painsaw is not rendered. I can't observe this with manual testing so I'd have to investigate more to provide a reliable repro.

Now I found a way. Set common.framerate.max 3 and fire the Painsaw once. On master you will see a frame with the realtime light rendered, but on this branch no. Maybe there is some issue with data being a frame out of date.

}

if ( _shader->UseMaterialSystem() && !_global ) {
if ( ( _shader->UseMaterialSystem() && _updateType == MATERIAL_OR_PUSH )
Copy link
Member

Choose a reason for hiding this comment

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

Helper function for this? It's a cryptic and unwieldy expression to have to repeat so many times.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reworked that part in #1685, it makes the whole uniform caching part much more straightforward.

@VReaperV VReaperV force-pushed the push-buffer-ubo branch 3 times, most recently from 4b840fd to 3e2071b Compare August 1, 2025 22:48
@VReaperV
Copy link
Contributor Author

VReaperV commented Aug 1, 2025

  1. Fog is not rendered if material system is enabled.

Fixed.

@VReaperV
Copy link
Contributor Author

VReaperV commented Aug 1, 2025

Now I found a way. Set common.framerate.max 3 and fire the Painsaw once. On master you will see a frame with the realtime light rendered, but on this branch no. Maybe there is some issue with data being a frame out of date.

Also fixed.

@slipher
Copy link
Member

slipher commented Aug 2, 2025

I confirm the 2 previous bugs are fixed. But I get the depth fade issue from #1676 again with this branch when enabling material system. It happens with any depth fade regardless of alpha tests, polygon offset etc. The bug reappears as of the first usable commit Add the remaining code for the global UBO in PushBuffer. (The first four commits don't build; GLUniform._global -> _updateType builds but doesn't render anything.)

@VReaperV
Copy link
Contributor Author

VReaperV commented Aug 2, 2025

I confirm the 2 previous bugs are fixed. But I get the depth fade issue from #1676 again with this branch when enabling material system. It happens with any depth fade regardless of alpha tests, polygon offset etc. The bug reappears as of the first usable commit Add the remaining code for the global UBO in PushBuffer. (The first four commits don't build; GLUniform._global -> _updateType builds but doesn't render anything.)

The comment there says it was already broken on material system? On my end it was working fine so I can't confirm if there's any issue with it on master right now.

@slipher
Copy link
Member

slipher commented Aug 2, 2025

It was broken but #1704 fixed it. It works on master.

@VReaperV
Copy link
Contributor Author

VReaperV commented Aug 3, 2025

It was broken but #1704 fixed it. It works on master.

Not sure what the issue is. I can't reproduce it on my end, and there don't seem to be any depth writes after glTextureBarrier().

@slipher
Copy link
Member

slipher commented Aug 4, 2025

The depth texture is all garbage all the time, as if it's not hooked up properly. It's still garbage if I set r_materialSystemSkip 1. It's still garbage with r_drawWorld 0 and nothing being rendered.

By the way, I realized you can make a depth texture visualization with just a q3shader, no GLSL required:

depthvis {
    {
        map _white
        depthFade 600
        blendFunc GL_SRC_ALPHA GL_ZERO
    }
}

@slipher
Copy link
Member

slipher commented Aug 4, 2025

Forgot screenshot
unvanquished_2025-08-03_212723_000

@VReaperV
Copy link
Contributor Author

VReaperV commented Aug 4, 2025

Hmm, the only thing I could think of is it has something to do with the textures being bindless. This is on AMD + Mesa, right? It might have the same cause as the broken occlusion culling reported there ~1 year ago.

@slipher
Copy link
Member

slipher commented Aug 4, 2025

This is on AMD + Mesa, right?

Yes

It might have the same cause as the broken occlusion culling reported there ~1 year ago.

On master if I turn on occlusion culling, it segfaults immediately.

@VReaperV
Copy link
Contributor Author

VReaperV commented Aug 4, 2025

On master if I turn on occlusion culling, it segfaults immediately.

Is the segfault coming from daemon? I don't get any on master.

@slipher
Copy link
Member

slipher commented Aug 4, 2025

I have provided a stack trace in #1724

@slipher
Copy link
Member

slipher commented Aug 5, 2025

Hmm, the only thing I could think of is it has something to do with the textures being bindless.

That's true, just r_preferBindlessTextures is enough to trigger the bug.

@VReaperV
Copy link
Contributor Author

VReaperV commented Aug 5, 2025

Hmm, the only thing I could think of is it has something to do with the textures being bindless.

That's true, just r_preferBindlessTextures is enough to trigger the bug.

Even with the fix from #1725?

@VReaperV
Copy link
Contributor Author

VReaperV commented Aug 5, 2025

Nevermind, if material system isn't enabled #1725 will have no effect.

@VReaperV
Copy link
Contributor Author

VReaperV commented Aug 5, 2025

Anyway, I think it's just a driver bug.

@VReaperV VReaperV force-pushed the push-buffer-ubo branch 2 times, most recently from 025efe7 to e0bf785 Compare August 12, 2025 05:56
Adds `PushBuffer` class, `pushBuffer` global, and the supporting code for `glconfig2`.
Also adds `GLShader.pushSkip` and `GLShader._pushUniforms`. Add `padding` back to the `GLShader` size calculations in material system.
Will be required for `PushBuffer` to set global uniform values outside of their shaders.
This will set `GLShaderManager.globalUniformBlock` to the struct + defines text, which will be the same for all shaders.
This will be required for `PushBuffer` to correctly sort uniforms. Also updates `GLShader.WriteUniformsToBuffer()` to use `mode` and `filter` arguments to select the correct uniforms.
Post-process shaders to actually add the `globalUniformBlock`, add `SetConstUniforms()` and `SetFrameUniforms()` functions to the core and material system renderers, and add the supporting glsl code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Renderer T-Feature-Request Proposed new feature T-Improvement Improvement for an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants