Skip to content

Simplify overbright implementation #1406

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

Merged
slipher merged 2 commits into from
Nov 14, 2024
Merged

Simplify overbright implementation #1406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5a4caf
Use half float instead of int16 for color buffer
slipher Nov 5, 2024
344932f
Simplify full-range overbright implementation
slipher Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 5 additions & 17 deletions src/engine/renderer/Material.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,15 +222,12 @@ void UpdateSurfaceDataGeneric3D( uint32_t* materials, Material& material, drawSu
// u_AlphaThreshold
gl_genericShaderMaterial->SetUniform_AlphaTest( pStage->stateBits );

// u_InverseLightFactor
float inverseLightFactor = pStage->cancelOverBright ? tr.mapInverseLightFactor : 1.0f;
gl_genericShaderMaterial->SetUniform_InverseLightFactor( inverseLightFactor );

// u_ColorModulate
colorGen_t rgbGen = SetRgbGen( pStage );
alphaGen_t alphaGen = SetAlphaGen( pStage );

gl_genericShaderMaterial->SetUniform_ColorModulate( rgbGen, alphaGen );
bool mayUseVertexOverbright = pStage->type == stageType_t::ST_COLORMAP && drawSurf->bspSurface;
gl_genericShaderMaterial->SetUniform_ColorModulate( rgbGen, alphaGen, mayUseVertexOverbright );

@illwieckz illwieckz Nov 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is preventing to multiply tess.svars.color in Tess_ComputeColor?

Why is the material variant calling Tess_ComputeColor but not the non-material one? Is that a dormant bug we missed?

@VReaperV VReaperV Nov 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-material code does Tess_ComputeColor() in Tess_StageIteratorColor(). Material system does not have that.

@illwieckz illwieckz Nov 13, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah so in non-material code, Tess_ComputeColor() is done before calling Render_generic3D(). Is it too early to multiply the light factor? Because both tess and pStage are known as well.

@illwieckz illwieckz Nov 13, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we can merge this as it is. Possible optimizations can be done later.

@VReaperV VReaperV Nov 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah so in non-material code, Tess_ComputeColor() is done before calling Render_generic3D(). Is it too early to multiply the light factor? Because both tess and pStage are known as well.

I suppose that was just to avoid having to add it to all relevant stage renderers.

@slipher slipher Nov 13, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What is preventing to multiply tess.svars.color in Tess_ComputeColor?

u_Color is not the same thing as u_ColorModulate. The formula for the "lighting" color (which will be multiplied by the diffuse) is u_ColorModulate * (vertex color) + u_Color. For vertex lighting, u_ColorModulate is 1, or 4 with overbright without lightmap, and u_Color is 0. Otherwise, u_ColorModulate is 0, so it's u_Color that needs to be scaled for light styles.


Tess_ComputeColor( pStage );
gl_genericShaderMaterial->SetUniform_Color( tess.svars.color );
Expand Down Expand Up @@ -291,12 +288,9 @@ void UpdateSurfaceDataLightMapping( uint32_t* materials, Material& material, dra
bool enableGridLighting = ( lightMode == lightMode_t::GRID );
bool enableGridDeluxeMapping = ( deluxeMode == deluxeMode_t::GRID );

// u_InverseLightFactor
/* HACK: use sign to know if there is a light or not, and
then if it will receive overbright multiplication or not. */
bool cancelOverBright = pStage->cancelOverBright || lightMode == lightMode_t::FULLBRIGHT;
float inverseLightFactor = cancelOverBright ? tr.mapInverseLightFactor : -tr.mapInverseLightFactor;
gl_lightMappingShaderMaterial->SetUniform_InverseLightFactor( inverseLightFactor );
// u_LightFactor
gl_lightMappingShaderMaterial->SetUniform_LightFactor(
lightMode == lightMode_t::FULLBRIGHT ? 1.0f : tr.mapLightFactor );

// u_ColorModulate
gl_lightMappingShaderMaterial->SetUniform_ColorModulate( rgbGen, alphaGen );
Expand Down Expand Up @@ -470,9 +464,6 @@ void UpdateSurfaceDataSkybox( uint32_t* materials, Material& material, drawSurf_
// u_AlphaThreshold
gl_skyboxShaderMaterial->SetUniform_AlphaTest( GLS_ATEST_NONE );

// u_InverseLightFactor
gl_skyboxShaderMaterial->SetUniform_InverseLightFactor( tr.mapInverseLightFactor );

gl_skyboxShaderMaterial->WriteUniformsToBuffer( materials );
}

Expand Down Expand Up @@ -620,9 +611,6 @@ void UpdateSurfaceDataFog( uint32_t* materials, Material& material, drawSurf_t*

const fog_t* fog = material.fog;

// u_InverseLightFactor
gl_fogQuake3ShaderMaterial->SetUniform_InverseLightFactor( tr.mapInverseLightFactor );

// u_Color
gl_fogQuake3ShaderMaterial->SetUniform_Color( fog->color );

Expand Down
26 changes: 6 additions & 20 deletions src/engine/renderer/gl_shader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2210,7 +2210,6 @@ GLShader_generic::GLShader_generic( GLShaderManager *manager ) :
u_AlphaThreshold( this ),
u_ModelMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_InverseLightFactor( this ),
u_ColorModulate( this ),
u_Color( this ),
u_Bones( this ),
Expand Down Expand Up @@ -2243,7 +2242,6 @@ GLShader_genericMaterial::GLShader_genericMaterial( GLShaderManager* manager ) :
u_AlphaThreshold( this ),
u_ModelMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_InverseLightFactor( this ),
u_ColorModulate( this ),
u_Color( this ),
u_DepthScale( this ),
Expand Down Expand Up @@ -2285,7 +2283,7 @@ GLShader_lightMapping::GLShader_lightMapping( GLShaderManager *manager ) :
u_ViewOrigin( this ),
u_ModelMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_InverseLightFactor( this ),
u_LightFactor( this ),
u_Bones( this ),
u_VertexInterpolation( this ),
u_ReliefDepthScale( this ),
Expand Down Expand Up @@ -2354,7 +2352,7 @@ GLShader_lightMappingMaterial::GLShader_lightMappingMaterial( GLShaderManager* m
u_ViewOrigin( this ),
u_ModelMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_InverseLightFactor( this ),
u_LightFactor( this ),
u_ReliefDepthScale( this ),
u_ReliefOffsetBias( this ),
u_NormalScale( this ),
Expand Down Expand Up @@ -2413,7 +2411,6 @@ GLShader_forwardLighting_omniXYZ::GLShader_forwardLighting_omniXYZ( GLShaderMana
u_ViewOrigin( this ),
u_LightOrigin( this ),
u_LightColor( this ),
u_InverseLightFactor( this ),
u_LightRadius( this ),
u_LightScale( this ),
u_LightAttenuationMatrix( this ),
Expand Down Expand Up @@ -2467,7 +2464,6 @@ GLShader_forwardLighting_projXYZ::GLShader_forwardLighting_projXYZ( GLShaderMana
u_ViewOrigin( this ),
u_LightOrigin( this ),
u_LightColor( this ),
u_InverseLightFactor( this ),
u_LightRadius( this ),
u_LightScale( this ),
u_LightAttenuationMatrix( this ),
Expand Down Expand Up @@ -2532,7 +2528,6 @@ GLShader_forwardLighting_directionalSun::GLShader_forwardLighting_directionalSun
u_ViewOrigin( this ),
u_LightDir( this ),
u_LightColor( this ),
u_InverseLightFactor( this ),
u_LightRadius( this ),
u_LightScale( this ),
u_LightAttenuationMatrix( this ),
Expand Down Expand Up @@ -2622,7 +2617,6 @@ GLShader_reflection::GLShader_reflection( GLShaderManager *manager ):
u_NormalScale( this ),
u_VertexInterpolation( this ),
u_CameraPosition( this ),
u_InverseLightFactor( this ),
GLDeformStage( this ),
GLCompileMacro_USE_VERTEX_SKINNING( this ),
GLCompileMacro_USE_VERTEX_ANIMATION( this ),
Expand Down Expand Up @@ -2651,7 +2645,6 @@ GLShader_reflectionMaterial::GLShader_reflectionMaterial( GLShaderManager* manag
u_ReliefOffsetBias( this ),
u_NormalScale( this ),
u_CameraPosition( this ),
u_InverseLightFactor( this ),
GLDeformStage( this ),
GLCompileMacro_USE_HEIGHTMAP_IN_NORMALMAP( this ),
GLCompileMacro_USE_RELIEF_MAPPING( this ) {
Expand All @@ -2673,8 +2666,7 @@ GLShader_skybox::GLShader_skybox( GLShaderManager *manager ) :
u_UseCloudMap( this ),
u_AlphaThreshold( this ),
u_ModelMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_InverseLightFactor( this )
u_ModelViewProjectionMatrix( this )
{
}

Expand All @@ -2694,9 +2686,8 @@ GLShader_skyboxMaterial::GLShader_skyboxMaterial( GLShaderManager* manager ) :
u_UseCloudMap( this ),
u_AlphaThreshold( this ),
u_ModelMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_InverseLightFactor( this ) {
}
u_ModelViewProjectionMatrix( this )
{}

void GLShader_skyboxMaterial::SetShaderProgramUniforms( shaderProgram_t* shaderProgram ) {
glUniform1i( glGetUniformLocation( shaderProgram->program, "u_ColorMap" ), 0 );
Expand All @@ -2708,7 +2699,6 @@ GLShader_fogQuake3::GLShader_fogQuake3( GLShaderManager *manager ) :
u_FogMap( this ),
u_ModelMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_InverseLightFactor( this ),
u_Color( this ),
u_Bones( this ),
u_VertexInterpolation( this ),
Expand All @@ -2731,7 +2721,6 @@ GLShader_fogQuake3Material::GLShader_fogQuake3Material( GLShaderManager* manager
u_FogMap( this ),
u_ModelMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_InverseLightFactor( this ),
u_Color( this ),
u_FogDistanceVector( this ),
u_FogDepthVector( this ),
Expand All @@ -2751,7 +2740,6 @@ GLShader_fogGlobal::GLShader_fogGlobal( GLShaderManager *manager ) :
u_ViewMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_UnprojectMatrix( this ),
u_InverseLightFactor( this ),
u_Color( this ),
u_FogDistanceVector( this ),
u_FogDepthVector( this )
Expand Down Expand Up @@ -2861,8 +2849,7 @@ void GLShader_portal::SetShaderProgramUniforms( shaderProgram_t *shaderProgram )
GLShader_contrast::GLShader_contrast( GLShaderManager *manager ) :
GLShader( "contrast", ATTR_POSITION, manager ),
u_ColorMap( this ),
u_ModelViewProjectionMatrix( this ),
u_InverseLightFactor( this )
u_ModelViewProjectionMatrix( this )
{
}

Expand All @@ -2878,7 +2865,6 @@ GLShader_cameraEffects::GLShader_cameraEffects( GLShaderManager *manager ) :
u_ColorModulate( this ),
u_TextureMatrix( this ),
u_ModelViewProjectionMatrix( this ),
u_LightFactor( this ),
u_DeformMagnitude( this ),
u_InverseGamma( this )
{
Expand Down
52 changes: 16 additions & 36 deletions src/engine/renderer/gl_shader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2150,21 +2150,6 @@ class u_LightFactor :
}
};

class u_InverseLightFactor :
GLUniform1f
{
public:
u_InverseLightFactor( GLShader *shader ) :
GLUniform1f( shader, "u_InverseLightFactor" )
{
}

void SetUniform_InverseLightFactor( const float inverseLightFactor )
{
this->SetValue( inverseLightFactor );
}
};

class u_ColorMap :
GLUniformSampler2D {
public:
Expand Down Expand Up @@ -3617,7 +3602,7 @@ class u_ColorModulate :
{
this->SetValue( v );
}
void SetUniform_ColorModulate( colorGen_t colorGen, alphaGen_t alphaGen )
void SetUniform_ColorModulate( colorGen_t colorGen, alphaGen_t alphaGen, bool vertexOverbright = false )
{
vec4_t v;
bool needAttrib = false;
Expand All @@ -3631,7 +3616,16 @@ class u_ColorModulate :
{
case colorGen_t::CGEN_VERTEX:
needAttrib = true;
VectorSet( v, 1, 1, 1 );
if ( vertexOverbright )
{
// vertexOverbright is only needed for non-lightmapped cases. When there is a
// lightmap, this is done by multiplying with the overbright-scaled white image
VectorSet( v, tr.mapLightFactor, tr.mapLightFactor, tr.mapLightFactor );
}
else
{
VectorSet( v, 1, 1, 1 );
}
break;

case colorGen_t::CGEN_ONE_MINUS_VERTEX:
Expand Down Expand Up @@ -3946,7 +3940,6 @@ class GLShader_generic :
public u_AlphaThreshold,
public u_ModelMatrix,
public u_ModelViewProjectionMatrix,
public u_InverseLightFactor,
public u_ColorModulate,
public u_Color,
public u_Bones,
Expand Down Expand Up @@ -3976,7 +3969,6 @@ class GLShader_genericMaterial :
public u_AlphaThreshold,
public u_ModelMatrix,
public u_ModelViewProjectionMatrix,
public u_InverseLightFactor,
public u_ColorModulate,
public u_Color,
public u_DepthScale,
Expand Down Expand Up @@ -4015,7 +4007,7 @@ class GLShader_lightMapping :
public u_ViewOrigin,
public u_ModelMatrix,
public u_ModelViewProjectionMatrix,
public u_InverseLightFactor,
public u_LightFactor,
public u_Bones,
public u_VertexInterpolation,
public u_ReliefDepthScale,
Expand Down Expand Up @@ -4067,7 +4059,7 @@ class GLShader_lightMappingMaterial :
public u_ViewOrigin,
public u_ModelMatrix,
public u_ModelViewProjectionMatrix,
public u_InverseLightFactor,
public u_LightFactor,
public u_ReliefDepthScale,
public u_ReliefOffsetBias,
public u_NormalScale,
Expand Down Expand Up @@ -4113,7 +4105,6 @@ class GLShader_forwardLighting_omniXYZ :
public u_ViewOrigin,
public u_LightOrigin,
public u_LightColor,
public u_InverseLightFactor,
public u_LightRadius,
public u_LightScale,
public u_LightAttenuationMatrix,
Expand Down Expand Up @@ -4157,7 +4148,6 @@ class GLShader_forwardLighting_projXYZ :
public u_ViewOrigin,
public u_LightOrigin,
public u_LightColor,
public u_InverseLightFactor,
public u_LightRadius,
public u_LightScale,
public u_LightAttenuationMatrix,
Expand Down Expand Up @@ -4208,7 +4198,6 @@ class GLShader_forwardLighting_directionalSun :
public u_ViewOrigin,
public u_LightDir,
public u_LightColor,
public u_InverseLightFactor,
public u_LightRadius,
public u_LightScale,
public u_LightAttenuationMatrix,
Expand Down Expand Up @@ -4275,7 +4264,6 @@ class GLShader_reflection :
public u_NormalScale,
public u_VertexInterpolation,
public u_CameraPosition,
public u_InverseLightFactor,
public GLDeformStage,
public GLCompileMacro_USE_VERTEX_SKINNING,
public GLCompileMacro_USE_VERTEX_ANIMATION,
Expand All @@ -4300,7 +4288,6 @@ class GLShader_reflectionMaterial :
public u_ReliefOffsetBias,
public u_NormalScale,
public u_CameraPosition,
public u_InverseLightFactor,
public GLDeformStage,
public GLCompileMacro_USE_HEIGHTMAP_IN_NORMALMAP,
public GLCompileMacro_USE_RELIEF_MAPPING {
Expand All @@ -4319,8 +4306,7 @@ class GLShader_skybox :
public u_UseCloudMap,
public u_AlphaThreshold,
public u_ModelMatrix,
public u_ModelViewProjectionMatrix,
public u_InverseLightFactor
public u_ModelViewProjectionMatrix
{
public:
GLShader_skybox( GLShaderManager *manager );
Expand All @@ -4337,8 +4323,7 @@ class GLShader_skyboxMaterial :
public u_UseCloudMap,
public u_AlphaThreshold,
public u_ModelMatrix,
public u_ModelViewProjectionMatrix,
public u_InverseLightFactor {
public u_ModelViewProjectionMatrix {
public:
GLShader_skyboxMaterial( GLShaderManager* manager );
void SetShaderProgramUniforms( shaderProgram_t* shaderProgram ) override;
Expand All @@ -4349,7 +4334,6 @@ class GLShader_fogQuake3 :
public u_FogMap,
public u_ModelMatrix,
public u_ModelViewProjectionMatrix,
public u_InverseLightFactor,
public u_Color,
public u_Bones,
public u_VertexInterpolation,
Expand All @@ -4370,7 +4354,6 @@ class GLShader_fogQuake3Material :
public u_FogMap,
public u_ModelMatrix,
public u_ModelViewProjectionMatrix,
public u_InverseLightFactor,
public u_Color,
public u_FogDistanceVector,
public u_FogDepthVector,
Expand All @@ -4389,7 +4372,6 @@ class GLShader_fogGlobal :
public u_ViewMatrix,
public u_ModelViewProjectionMatrix,
public u_UnprojectMatrix,
public u_InverseLightFactor,
public u_Color,
public u_FogDistanceVector,
public u_FogDepthVector
Expand Down Expand Up @@ -4484,8 +4466,7 @@ class GLShader_portal :
class GLShader_contrast :
public GLShader,
public u_ColorMap,
public u_ModelViewProjectionMatrix,
public u_InverseLightFactor
public u_ModelViewProjectionMatrix
{
public:
GLShader_contrast( GLShaderManager *manager );
Expand All @@ -4499,7 +4480,6 @@ class GLShader_cameraEffects :
public u_ColorModulate,
public u_TextureMatrix,
public u_ModelViewProjectionMatrix,
public u_LightFactor,
public u_DeformMagnitude,
public u_InverseGamma
{
Expand Down
4 changes: 0 additions & 4 deletions src/engine/renderer/glsl_source/cameraEffects_fp.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ uniform sampler2D u_CurrentMap;
uniform sampler3D u_ColorMap3D;
#endif

uniform float u_LightFactor;

uniform vec4 u_ColorModulate;
uniform float u_InverseGamma;

Expand All @@ -44,8 +42,6 @@ void main()

vec4 color = texture2D(u_CurrentMap, st);

color.rgb *= u_LightFactor;

color = clamp(color, 0.0, 1.0);

#if defined(r_colorGrading)
Expand Down
Loading