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.

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

Fix glsl_restart with material system #1507

Merged
merged 1 commit into from
Feb 3, 2025
Merged

Fix glsl_restart with material system #1507

merged 1 commit into from
Feb 3, 2025

Conversation

VReaperV
Copy link
Contributor

@VReaperV VReaperV commented Jan 17, 2025 •
edited
Loading

Fixes glsl_restart with material system by resetting the shaders for each material.

@VReaperV VReaperV force-pushed the fix-glsl-restart branch 2 times, most recently from 0d00485 to e639ae3 Compare January 21, 2025 02:16
@VReaperV VReaperV mentioned this pull request Jan 22, 2025
Merged
@VReaperV VReaperV force-pushed the fix-glsl-restart branch 7 times, most recently from fddf9d2 to 275e96b Compare January 28, 2025 22:01
@VReaperV
Copy link
Contributor Author

Made it just reset shaders rather than rebuild all materials, and rebased on master.

@VReaperV
Copy link
Contributor Author

Appveyor failed becuase of some appveyor or github error:

fatal: unable to access 'https://github.com/DaemonEngine/Daemon.git/': TLS connect error: error:00000000:lib(0)::reason(0)
Command exited with code 128

@illwieckz
Copy link
Member

Yeah, AppVeyor is having trouble to connect to GitHub right now, I restarted the build 4 times before getting it finished, and I faced similar issues at different times like that:

Cloning into 'C:/projects/daemon/libs/pdcursesmod'...
Cloning into 'C:/projects/daemon/libs/breakpad'...
fatal: unable to access 'https://github.com/DaemonEngine/breakpad.git/': TLS connect error: error:00000000:lib(0)::reason(0)
fatal: clone of 'https://github.com/DaemonEngine/breakpad.git' into submodule path 'C:/projects/daemon/libs/breakpad' failed

git clone -q https://github.com/DaemonEngine/Daemon.git C:\projects\daemon
fatal: unable to access 'https://github.com/DaemonEngine/Daemon.git/': TLS connect error: error:00000000:lib(0)::reason(0)

slipher
slipher reviewed Jan 29, 2025
View reviewed changes
Copy link
Member

@slipher slipher left a comment

Choose a reason for hiding this comment

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

To make the drawSurf_t copying hack less likely to go awry, I propose zeroing its pointer members which may be invalidated, of type trRefEntity_t * and drawSurf_t *. That way we will get a clean crash if one of them ends up being used.

src/engine/renderer/Material.h
@@ -85,6 +85,10 @@ struct Material {
GLuint program = 0;
GLShader* shader;

// Used only for glsl_restart
shaderStage_t* refStage;
drawSurf_t refDrawSurf;
Copy link
Member

Choose a reason for hiding this comment

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

What does "ref" mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reference.

Copy link
Member

Choose a reason for hiding this comment

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

refDrawSurf isn't a reference though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, as in "reference to the values used for this material". Might not be a good name for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something like original[DrawSurf/Stage] might be better.

@VReaperV VReaperV force-pushed the fix-glsl-restart branch 2 times, most recently from 14dc0d3 to 5e12a8c Compare January 30, 2025 00:51
@VReaperV
Copy link
Contributor Author

To make the drawSurf_t copying hack less likely to go awry, I propose zeroing its pointer members which may be invalidated, of type trRefEntity_t * and drawSurf_t *. That way we will get a clean crash if one of them ends up being used.

Good point, done now.

@slipher
Copy link
Member

slipher commented Feb 3, 2025

LGTM

@VReaperV VReaperV merged commit 2925177 into DaemonEngine:master Feb 3, 2025
9 checks passed
@VReaperV VReaperV deleted the fix-glsl-restart branch February 3, 2025 11:29
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-Bug
Projects
Material system
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@VReaperV @illwieckz @slipher