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

[rant] XreaL material syntax is broken by design #216

Open
illwieckz opened this issue Jul 24, 2019 · 3 comments

Comments

@illwieckz
Copy link
Member

commented Jul 24, 2019

Recently I had some talk with @xycaleth from @JACoders, and he taughth me that ioq3 and OpenJK relies on normalFormat keyword to handle channel flipping (for example: flipping the Y channel to convert from OpenGL format to DirectX format). Basically you can do normalFormat 2 -2 2 to double all the channels and revert the Y one. This is clever.

Previously I implemented a normalFormat keyword in Dæmon with purpose to flip normal map channels with. At the same time, I discovered that normal map scaling using shader keywords was not implemented (only a cvar existed, with only one value for both X and Y).

So I thought about implementing normalScale and ditch normalFormat. I implemented normalScale on stage level, because it's where it is expected to be done. Then I added the normalFormat -1 1 1 line on a vega shader that is known to have a wrong normal map with flipped X channel. Then I wanted to test my code and NOOOOOOOOOOO. Please, XreaL, get out of my way! %@#§ø%!!!

The edited shader was looking like that:

textures/shared_vega/panel01
{
	qer_editorImage textures/shared_vega_src/panel01_d

	diffuseMap  textures/shared_vega_src/panel01_d
	normalMap   textures/shared_vega_src/panel01_n
	specularmap textures/shared_vega_src/panel01_s

	normalScale -1 1 1
}

But that can't work because the keyword is not in a stage, but it would be very ugly to implement the keyword on a shader level instead! :'(

So, what do we know about that %@#§ø% Xreal shader syntax ?

We know that all those diffuseMap, normalMap lines are shortcuts for stages, stages that are then collapsed with some code that is hard to edit without leaving it half broken.

So I would be able to do that:

textures/shared_vega/panel01
{
	qer_editorImage textures/shared_vega_src/panel01_d

	diffuseMap  textures/shared_vega_src/panel01_d
	{
		stage       normalMap
		map         textures/shared_vega_src/panel01_n
		normalScale	-1 1 1
	}
	specularmap textures/shared_vega_src/panel01_s
}

But that's ugly.

There is another way to do shaders, and that would magically solve the need for a collapsing code, and would solve my need for the normalScale keyword, and that would make Dæmon able to do terrain blending with normal and specular maps one day !

Because we can do that today:

textures/thunder_custom/ter_rocksand_xy
{
	qer_editorImage textures/thunder_custom_src/ter_rocksand_p
	q3map_nonplanar
	q3map_shadeAngle 90
	q3map_tcGen ivector ( 256 0 0 ) ( 0 256 0 )
	q3map_alphaMod dotproduct2 ( 0 0 .75 )

	{
		map textures/shared_pk02_src/rock01_d
		rgbGen identity
	}
	{
		map textures/shared_pk02_src/sand01_d
		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
	}
}

But we can only blend colormaps, there is no way to add normalMap since the XreaL-inherited code expects the normalMap to be a stage that would be collapsed by the engine, and there is no way to tell the engine to collapse the normal map to one of the colormaps used in that terrain blending keyword.

What I suggest that instead of doing that:

textures/shared_pk01/door01a_norad
{
	qer_editorImage     textures/shared_pk01_src/door01a_d

	diffuseMap          textures/shared_pk01_src/door01a_d
	normalMap           textures/shared_pk01_src/door01_n
	specularMap         textures/shared_pk01_src/door01_s
	glowMap             textures/shared_pk01_src/door01_a
}

We do that:

textures/shared_pk01/door01a_norad
{
	qer_editorImage     textures/shared_pk01_src/door01a_d
	{
		diffuseMap          textures/shared_pk01_src/door01a_d
		normalMap           textures/shared_pk01_src/door01_n
		specularMap         textures/shared_pk01_src/door01_s
		glowMap             textures/shared_pk01_src/door01_a
	}
}

That would allow us to do that:

textures/shared_vega/panel01
{
	qer_editorImage textures/shared_vega_src/panel01_d

	{
		diffuseMap  textures/shared_vega_src/panel01_d
		normalMap   textures/shared_vega_src/panel01_n
		specularmap textures/shared_vega_src/panel01_s
		normalScale -1 1 1
	}
}

As soon as we don't leave the stage, the engine would collapse stages without having to know clever stuff.

And we would be able to do that:

textures/thunder_custom/ter_rocksand_xy
{
	qer_editorImage textures/thunder_custom_src/ter_rocksand_p
	q3map_nonplanar
	q3map_shadeAngle 90
	q3map_tcGen ivector ( 256 0 0 ) ( 0 256 0 )
	q3map_alphaMod dotproduct2 ( 0 0 .75 )

	{
		diffuseMap  textures/shared_pk02_src/rock01_d
		normalMap   textures/shared_pk02_src/rock01_n
		specularMap textures/shared_pk02_src/rock01_d
		rgbGen identity
		nolightmap
	}
	{
		diffuseMap  textures/shared_pk02_src/sand01_d
		normalMap   textures/shared_pk02_src/sand01_n
		specularMap textures/shared_pk02_src/sand01_s
		blendFunc GL_SRC_ALPHA GL_ONE_MINUS_SRC_ALPHA
		alphaFunc GE128
		rgbGen identity
		alphaGen vertex
		nolightmap
	}
	{
		map $lightmap
		blendFunc GL_DST_COLOR GL_ZERO
		rgbGen identity
	}
}

So, yes, we would have to convert our existing shaders, but it's just a matter of adding one { and one }. I'm very OK to do it.

We would have to edit Sloth to add those {, } too. It may be not so hard to do.

In the end the code will be cleaner and features would be easier to implement. We would also be able to deduplicate all the if keyword is diffuseMap or normaMap or… code that lives both in shader parser and stage parser, to only keep the stage parser.

%@#§ø%!!!

@illwieckz illwieckz changed the title [rant] xreal material syntax is broken by design [rant] XreaL material syntax is broken by design Jul 24, 2019

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2019

@Viech, @DolceTriade, what do you think about it?

@DolceTriade

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

I'm ok with the idea in general.

@illwieckz

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

I plan this for after 0.52 in anyway

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