Skip to content

Collapse shaderType_t::3D_DYNAMIC and 3D_STATIC into one #1699

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

Merged
merged 3 commits into from
Jul 21, 2025

Conversation

slipher
Copy link
Member

@slipher slipher commented Jul 16, 2025

There was no difference in behavior between them.

slipher added 3 commits July 15, 2025 23:13
RE_RegisterShaderFromImage creates a q3shader object for textures that
have just been generated at runtime. Don't attempt to find
them in the shader hash table. This doesn't make much sense and even if
a match were found, an image was already uploaded at that point so the
memory is already wasted.
SHADER_3D_DYNAMIC and SHADER_3D_STATIC were two options for implicitly
creating a shader from an image that lacks a text shader. Merge them
into a single SHADER_3D.

The only distinction between the two was using CGEN_IDENTITY vs.
CGEN_IDENTITY_LIGHTING. This only makes a difference if r_overbrightQ3
is on. But even for emulating Quake 3 the code was wrong: it requested
CGEN_IDENTITY_LIGHTING, which cancels overbright, for models. Models use
the light grid which *should* receive the overbright factor. Actual ioq3
uses CGEN_LIGHTING_DIFFUSE, which we no longer have, for those models.
Copy link
Member

@illwieckz illwieckz left a comment

Choose a reason for hiding this comment

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

LGTM.

@slipher slipher merged commit 796e130 into DaemonEngine:master Jul 21, 2025
9 checks passed
@slipher slipher deleted the shader-type-simplify branch July 21, 2025 08:46
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