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.

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

tr_shader: color map after diffuse map keeps light map explicitely ordered #197

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
1 participant
@illwieckz
Copy link
Member

commented Apr 7, 2019

I'm not sure this is a good idea: keep color map after diffuse map ordered after lightmap, some maybe mistake, others may be legacy addition map (glow map).

because of the bad design of the *Map material stuff there it's hard to raise a warning when people mistakenly put color maps after diffuse map, meaning a proper collapsing code paints the lightmap before the color maps overriding them, we can see an example of bad material like this in hangar28 map by tvezet:

textures/hangar28_pk02/sand_stone
{
	qer_editorimage textures/shared_pk02_src/sand01_d
	
	q3map_nonplanar
	q3map_shadeangle 170
	q3map_lightmapmergable
	
	diffuseMap          textures/shared_pk02_src/sand01_d
	normalMap           textures/shared_pk02_src/sand01_n
	specularMap         textures/shared_pk02_src/sand01_s
    
	{
		map textures/shared_pk02_src/rock01_d	// Primary
		rgbGen identity
	}
	{
		map textures/shared_pk02_src/sand01_d	// Secondary
		blendFunc GL_SRC_ALPHA GL_ONE_MINUS_SRC_ALPHA
		//alphaFunc GE128
		rgbGen identity
		alphaGen vertex
		
	}
	{
		map $lightmap
		blendFunc GL_DST_COLOR GL_ZERO
		rgbGen identity
	}
}

We see a collapsable diffuse stage then two color maps in a terrain blending pattern then a lightmap. In such case lightmap must be disabled in collapsable diffuse stage to be sure the explicit lightmap stage is painted over the terrain map. One would say, “why take care of such monstruosity?” the problem is that legacy addition map just looks the same for a parser point of view, for example our medistation material has a legacy addition map:

models/buildables/medistat/medistat
{
	qer_editorImage models/buildables/medistat/medistat_d
	diffuseMap models/buildables/medistat/medistat_d
	normalMap models/buildables/medistat/medistat_n
	{
		stage specularMap
		map models/buildables/medistat/medistat_s
		specularExponentMin 10
		specularExponentMax 25
	}
	{
		map models/buildables/medistat/medistat_a
		blendfunc add
	}
}

Hopefully there is no explicit lightmap stage so we are lucky and lightmap is already painted before the addition map, but add an explicit lightmap stage with a custom filtering and then you get bugs. Note that those materials may be rewritten to use the glowMap keyword to avoid one rendering stage but that's out of topic.

In some case the legacy addition maps can't use the glowMap keyword, especially when there is more than one and/or they have specific effects. See that drill example:

models/buildables/drill/drill
{
	qer_editorImage models/buildables/drill/drill_d
	diffuseMap models/buildables/drill/drill_d
	normalMap models/buildables/drill/drill_n
	specularMap models/buildables/drill/drill_s
	// white lamp on top
	{
		map models/buildables/drill/drill_top_a
		blendfunc add
		rgbGen    wave sin 1 .85 .5 .08
	}
	// small yellow lamps around
	{
		map models/buildables/drill/drill_around_a
		blendfunc add
		rgb       .85 .85 .85
	}

	when destroyed models/buildables/drill/drill_dead
}

In this example there is no explicit lightmap stage, but if there was we have to make sure they are ordered well. Note that I haven't found yet unproper legacy addition map with bad placed light stage, and by luck the light stage is implicitely painted before the legacy addition map hence it's already rendered properly when lightmap stage is implicit.

Not merging this code would keep the hangar28 bug obvious to mappers which is not bad. I want to be sure there is no legit case for such pattern. If there is no legit case for such pattern (i.e. colormap that is not an addition map after collapsable diffuse stage with explicit lightmap stage), there is no need to merge that. Map would have to be fixed in any way.

Note that this commit also adds a test for stages being active when iterating them to detect their kind, this minor change can be merged in any way and I will extract it as a standalone commit.

@illwieckz illwieckz changed the title tr_shader: keep color map after diffuse map ordered after lightmap tr_shader: keep color map after diffuse map ordered before lightmap Apr 7, 2019

@illwieckz illwieckz force-pushed the illwieckz:keepcolormap branch from 72ac7fe to 61aa8ce Apr 7, 2019

@illwieckz illwieckz changed the title tr_shader: keep color map after diffuse map ordered before lightmap tr_shader: color map after diffuse map keep light map explicitely ordered Apr 7, 2019

@illwieckz illwieckz force-pushed the illwieckz:keepcolormap branch from 61aa8ce to 75d68c6 Apr 7, 2019

@illwieckz illwieckz changed the title tr_shader: color map after diffuse map keep light map explicitely ordered tr_shader: color map after diffuse map keeps light map explicitely ordered Apr 7, 2019

@illwieckz illwieckz force-pushed the illwieckz:keepcolormap branch 2 times, most recently from b2ac9cb to ecfdb1e Apr 7, 2019

illwieckz added some commits Apr 7, 2019

tr_shader: color map after diffuse map keeps light map explicitely or…
…dered

some may be mistakes, others may be legacy addition maps (glow maps)

@illwieckz illwieckz force-pushed the illwieckz:keepcolormap branch from ecfdb1e to f3856d5 Apr 7, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2019

Well, the more I think about it, the more I think only wrong shader can show this pattern.

Also, legacy maps didn't use the diffuseMap keyword so it's not a matter of supporting legacy stuff, only broken stuff.

I noticed that some legacy shader use various lightmap blending modes. If there is really more than one lightmap blending operation, then we may have to merge this code, because it would be legit to set a diffuse map then a custom lightmap stage with a non-standard blending operation, then a non-standard addition map.

Note that I would like to see the lightMapping glsl shader supporting the alternate lightmap blend operations to definitely kill by fire the standalone lightmap stage with collapsible diffuseMap one. This supports would also make this code unneeded (just leaving broken pattern as broken).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.