Skip to content

Only update PushBuffer if any uniform has actually changed #1685

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.

Sign up for GitHub

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

Jump to bottom
Open
VReaperV wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Only update PushBuffer if any uniform has actually changed #1685

VReaperV wants to merge 4 commits into from
+130 −401

Conversation

@VReaperV
Copy link
Contributor

@VReaperV VReaperV commented Jun 23, 2025

Requires #1684

Further improvement for PushBuffer, only update it if it's actually required.

@VReaperV VReaperV added T-Improvement Improvement for an existing feature A-Renderer labels Jun 23, 2025
@VReaperV VReaperV force-pushed the push-buffer-update branch 2 times, most recently from bec3aa0 to ad58d5c Compare June 24, 2025 17:42
@VReaperV
Copy link
Contributor Author

VReaperV commented Jun 24, 2025

Also added a fix for globalFog fog texture.

@VReaperV VReaperV force-pushed the push-buffer-update branch 2 times, most recently from f67e29b to e84d583 Compare June 25, 2025 05:55
@VReaperV
Copy link
Contributor Author

VReaperV commented Jun 25, 2025 •
edited
Loading

Changed the initial commit and added a new one, now it mutualises currentValue and uniformFirewall (in a way that works with any kind of uniform unlike uniformFirewall, and with both buffers and shader programs unlike currentValue), GLUniform.WriteToBuffer() now only has 1 definition, and the uniform setting functions are now much simpler. Also NUKED a bunch of now unused code.

@VReaperV VReaperV force-pushed the push-buffer-update branch from 80717b6 to 8832f45 Compare June 30, 2025 15:27
@VReaperV VReaperV mentioned this pull request Jul 25, 2025
Merged
@VReaperV VReaperV force-pushed the push-buffer-update branch 3 times, most recently from fedb60c to fc2fff3 Compare August 1, 2025 22:59
@VReaperV VReaperV force-pushed the push-buffer-update branch from fc2fff3 to bf51f59 Compare August 19, 2025 11:07
This was referenced Aug 27, 2025
Closed
Merged
@VReaperV VReaperV force-pushed the push-buffer-update branch 2 times, most recently from 9a82e42 to 553f6d9 Compare January 9, 2026 01:40
slipher
slipher reviewed Jan 11, 2026
View reviewed changes
src/engine/renderer/gl_shader.h
public:
u_Bones( GLShader *shader ) :
GLUniform4fv( shader, "u_Bones", MAX_BONES, PUSH )
GLUniform4fv( shader, "u_Bones", MAX_BONES * 2, PUSH )
Copy link
Member

@slipher slipher Jan 11, 2026

Choose a reason for hiding this comment

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

Why times two?

Copy link
Contributor Author

@VReaperV VReaperV Jan 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Because the bone count is multiplied by 2 in SetUniform_Bones().

src/engine/renderer/gl_shader.h
if ( ( _shader->UseMaterialSystem() && _updateType == MATERIAL_OR_PUSH )
|| ( glConfig.pushBufferAvailable && _updateType <= FRAME ) ) {
memcpy( currentValue, m, 6 * sizeof( float ) );
if ( !CacheValue( value ) ) {
Copy link
Member

@slipher slipher Jan 11, 2026

Choose a reason for hiding this comment

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

I think this could give an incorrect result if called with differing transpose parameters.

Copy link
Contributor Author

@VReaperV VReaperV Jan 12, 2026

Choose a reason for hiding this comment

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

Yeah, but why would you do that? Besides, using this transpose arg wouldn't work correctly with material or push buffers.

@VReaperV VReaperV force-pushed the push-buffer-update branch 3 times, most recently from 54c1107 to 36eefa5 Compare January 12, 2026 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slipher slipher slipher left review comments

Assignees

No one assigned

Labels

A-Renderer T-Improvement Improvement for an existing feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@VReaperV @slipher