Skip to content

Conversation

slipher
Copy link
Member

@slipher slipher commented Aug 1, 2025

Implement stage-level colorspace keyword as described in #1717 (comment)

Also add a warning if some shaders are loaded too early before the colorspace is known.

@slipher slipher changed the title Stage colorspace Stage-level colorspace keyword for shaders Aug 1, 2025
@slipher
Copy link
Member Author

slipher commented Aug 3, 2025

Maybe I should add "if" to the keyword name to make it clearer that it is for conditional stages. Like ifBlendspace linear.

Since the necessity of sRGB conversions is not known.
Allows making conditional stages that depend on whether naive or linear
blending is used.
@slipher
Copy link
Member Author

slipher commented Oct 13, 2025

Renamed the keyword from colorspace to ifBlendRegime.

@illwieckz
Copy link
Member

illwieckz commented Oct 16, 2025

I believe I found an use case for this!

In tex-trak5, the textures/shared_trak5/glass shader still looks wrong with the naiveColors trick:

Naive build, naive pipeline:

unvanquished_2025-10-16_133156_000

Linear build, linear pipeline:

unvanquished_2025-10-16_133327_000

Linear build, linear pipeline, naiveColors trick on the glass:

unvanquished_2025-10-16_133413_000

We need to do some alpahGen or something like that to recover the translucency of it, but we only need it with the linear pipeline, and this is a texture set that can be used by legacy maps.

@illwieckz
Copy link
Member

For the if *** implementation, here is an example of prior art (Warsow) as an example of syntax:

textures/grates/fence02
{	
	qer_trans 0.5
	qer_editorimage textures/grates/fence02.tga
	surfaceparm trans
	surfaceparm nomarks
	surfaceparm alphashadow
	cull none
	nopicmip
	nomipmaps
	q3map_forceMeta
	q3map_lightmapsamplesize 64
	
if deluxe
	{
		material textures/grates/fence02.tga textures/grates/fence01_norm.tga textures/grates/fence01_gloss.tga
		alphaFunc GE128
		blendFunc blend
		depthWrite
	}
endif

if ! deluxe
	{
		map textures/grates/fence02.tga
		blendfunc blend
		alphaFunc GE128
		depthWrite
	}

	{
		map $lightmap
		rgbGen identity
		blendFunc filter
		depthFunc equal
	}
endif
}

@slipher
Copy link
Member Author

slipher commented Oct 16, 2025

I don't see the benefit of adding a new type of syntactical construct to the shader grammar like that. (Also would it break q3map2?) Having stage enablement conditions inside the stage is more consistent with prior art in the history of our engine. For example the detail keyword from Q3A, used in some ET maps, is a keyword that may disable the stage at load time depending on a cvar. Also we have an if stage-level keyword which can disable the stage at runtime and is evaluated every time the shader is rendered.

It's true that it may be useful to allow other conditions beside blend regime. if is already taken as described above, so for a general-purpose load time condition we'd have to call it something else like staticIf. Or we could just NUKE or rename the runtime if since it has never been used in a map per https://users.unvanquished.net/~slipher/shader-keyword-directory.txt.

@illwieckz
Copy link
Member

illwieckz commented Oct 16, 2025

I don't see the benefit of adding a new type of syntactical construct to the shader grammar like that.

That syntax completely skips the stage parsing.

Also would it break q3map2?

Q3map2 only reads q3map_* keywords, the qer_editorImage one, cull and the surfaceparms and skips everything else.

Warsow uses q3map2.

if is already taken as described above

Within stages, not outside stages.

@illwieckz
Copy link
Member

Also we have an if stage-level keyword which can disable the stage at runtime and is evaluated every time the shader is rendered.

We don't need run time switching for this specific use case.

@slipher
Copy link
Member Author

slipher commented Oct 16, 2025

That syntax completely skips the stage parsing.

I see that as a bad thing. I would rather parse all the stages all the time, rather than do it C preprocessor style and allow any garbage in the stage. That way designers get immediate feedback if they make a syntax error, regardless of configuration. We can avoid unnecessary image loading by making the image loading lazier as has been discussed recently. (But this is not an urgent feature since I expect very few shaders with distinct images depending on condition.) For the image loading I think the ideal would be that it immediately checks whether a matching file exists, but does not open the file until the shader is determined to be otherwise valid.

Regarding "if", I was discussing the possibility of a stage-level, load-time keyword. The point is, given that there is already an if which switches at runtime, we have to make a different keyword since we want to switch at load time.

@illwieckz
Copy link
Member

illwieckz commented Oct 16, 2025

Regarding "if", I was discussing the possibility of a stage-level, load-time keyword. The point is, given that there is already an if which switches at runtime, we have to make a different keyword since we want to switch at load time.

We can probably also use if static <condition>, since we don't use that if yet, we can extend the syntax without breaking it. Otherwise staticIf or something like that would do it fine.

For your knowledge, that if syntax comes from Doom 3. The fact we don't use it yet doesn't mean we don't want to use it. We have features we may appreciate but simply don't use yet because we don't know about them (like the trick of using heatHazeMap on glass surfaces we only started to use very recently…).

@illwieckz
Copy link
Member

An example of complex material using if from Doom 3 (taken from materials/lights.mtr):

lights/fanlightgrateSC
{
	{
		forceHighQuality
		if ( global0 == 0 )
		map		lights/fangrate.tga
		colored
		zeroclamp
	}
	{
		forceHighQuality
		if ( global0 == 0 )
		map	lights/fanblade.tga
		colored
		zeroclamp
		rotate	time * -1
	}
	{
		if ( global0 != 0 )
		forceHighQuality
		map		lights/fangrate.tga
		red	( Parm0 * acceleratorflashtable2[ ( time - global1 ) / 7 ] )
		green	( Parm1 * acceleratorflashtable2[ ( time - global1 ) / 7 ] )
		blue	( Parm2 * acceleratorflashtable2[ ( time - global1 ) / 7 ] )
		zeroclamp
	}
	{
		if ( global0 != 0 )
		forceHighQuality
		map	lights/fanblade.tga
		red	( Parm0 * acceleratorflashtable2[ ( time - global1 ) / 7 ] )
		green	( Parm1 * acceleratorflashtable2[ ( time - global1 ) / 7 ] )
		blue	( Parm2 * acceleratorflashtable2[ ( time - global1 ) / 7 ] )
		zeroclamp
		rotate	time * -1
	}
}

@slipher
Copy link
Member Author

slipher commented Oct 16, 2025

An example of complex material using if from Doom 3 (taken from materials/lights.mtr):

I think our engine is able to parse this. It appears to be intended for runtime switching. However in our engine global0 is hardwired to be always 1.0.

Note that for this kind of runtime switching (for example powering on or off buildables), our assets use the when keyword, which switches to an entirely new shader rather than modifying individual stages.

Anyway, for the stage-level condition, one more option I haven't mentioned is that we could analyze the expression to see if it is a constant or not, thus reusing if for static or runtimes conditions.

I dislike the Warsow preprocessor-style syntax, but for stage level stuff whatever syntax is fine. Which way do you think is best? The current ifBlendRegime? An ifStatic that would parse some expression like ifStatic linearBlending? Or the same reusing the if keyword like if linearBlending and automatic detection of whether the condition is constant?

@illwieckz
Copy link
Member

Anyway, for the stage-level condition, one more option I haven't mentioned is that we could analyze the expression to see if it is a constant or not, thus reusing if for static or runtimes conditions.

That's something I would like to see but I don't want to delay such PR because of that.

Which way do you think is best? The current ifBlendRegime? An ifStatic that would parse some expression like ifStatic linearBlending? Or the same reusing the if keyword like if linearBlending and automatic detection of whether the condition is constant?

We can start with staticIf linearBlending and we may later make it an alias for if if we implement static condition detection later.

Or we may find another keyword like require linearBlending and require !linearBlending.

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