Yet another overbright bug (deformvertexes autosprite2)

[slipher]

Map: https://users.unvanquished.net/~sweet/pkg/map-defenxe_0+b2.dpk

Shader:

textures/mario/tree_shader
{
	qer_editorimage textures/mario/tree.tga
	surfaceparm alphashadow
	surfaceparm noimpact
	surfaceparm nomarks
	surfaceparm nonsolid
	surfaceparm trans
	cull disable
	deformVertexes autosprite2
	{
		map textures/mario/tree.tga
		rgbGen identity
		depthWrite
		alphaFunc GE128
	}
	{
		map $lightmap 
		blendfunc filter
		rgbGen identity
		tcGen lightmap 
		depthFunc equal
	}
}

How it now looks:

With r_forcelegacyoverbrightclamping 1, all the trees looks like the brightly colored one under the map name in the levelshot:

(There is a second bug that unlike the levelshot, the trees are seemingly unaffected by the light grid and all equally bright. But that's unchanged since 0.51.)

Also there is a third bug that the tree sprites are incorrectly culled despite the shader saying cull disable :)

[illwieckz]

Alongside overbright and culling problems, maybe there are other problems involved. The engine currently assume a BSP surface is fullbright if there is no lightmap, it only uses lightgrid for non-BSP models, but maybe such surface have no lightmap and maybe they should use lightgrid.

I want to use lightgrid on some bsp surfaces while the rest of the world use lightmap, especially for moving parts like doors. Maybe the original quake3 engine already did this and maybe those trees are an example of this. This is also something I thought about some autosprite2 in metro (chains not being properly lit).

And of course, among the various problems, this trees are autosprite2 and autosprite2 is still incomplete:

• https://github.com/DaemonEngine/Daemon/issues/730

[slipher]

I misspoke saying lightgrid, I believe it should just be lightmaps in this case. Although lightmaps are not a theoretically correct approach here, considering the surface can be drawn at different positions/angles.

The problem is that gl_lightMappingShader does not know how to fetch sprite vertexes. It is not configured with the USE_VERTEX_SPRITE macro. So the lightmap vertexes are not drawn at the correct position; this causes both the first and second bugs. So apparently in the new overbright implementation, the color map stage draws the image with 4x brightness or whatever, and needs the lightmap stage to cut that down to something reasonable. While with r_forcelegacyoverbrightclamping 1 it starts out with 1x brightness so it will not look so bad when the lightmap stage fails to draw anything.

I tried a bit to set it up the lightmap shader with USE_VERTEX_SPRITE and the needed uniforms, but still couldn't get the vertexes to appear in the correct location. But anyway, the whole VBO sprite vertexes thing smells like bad design. We have 4 different options for reading out a vertex in GLSL: vertexSimple_vp.glsl, vertexAnimation_vp.glsl, vertexSkinning_vp.glsl, and vertexSprite_vp.glsl. So for any GLSL shader that needs to support all the variants, the number of GLSL binaries is multiplied by 4. The first variant is a basic vertex with nothing fancy. The 2nd and 3rd are specialized for types of models. It makes sense to have these because models are expensive to draw and used a lot. But the 4th is only useful for being able to draw BSP autosprite surfaces from the static VBO. This is not compelling because the feature is seldom used. Moreover the number of triangles is small: sprites are inherently low-poly because the surface is required to be a rectangle composed of 2 triangles to work. So I propose NUKING USE_VERTEX_SPRITE/vertexSprite_vp.glsl and just adding the vertexes from the CPU. Essentially, reverting b10e8c278c7817d52b6095f9f961598792a04dcb.

I might want to do this before finishing my vertex translation branch since it would get rid of the nasty case of two overlapping vertex attribute arrays for ATTR_ORIENTATION and ATTR_QTANGENT (see the union in shaderVertex_t).

[VReaperV]

I tried a bit to set it up the lightmap shader with USE_VERTEX_SPRITE and the needed uniforms, but still couldn't get the vertexes to appear in the correct location. But anyway, the whole VBO sprite vertexes thing smells like bad design. We have 4 different options for reading out a vertex in GLSL: vertexSimple_vp.glsl, vertexAnimation_vp.glsl, vertexSkinning_vp.glsl, and vertexSprite_vp.glsl. So for any GLSL shader that needs to support all the variants, the number of GLSL binaries is multiplied by 4. The first variant is a basic vertex with nothing fancy. The 2nd and 3rd are specialized for types of models. It makes sense to have these because models are expensive to draw and used a lot. But the 4th is only useful for being able to draw BSP autosprite surfaces from the static VBO. This is not compelling because the feature is seldom used. Moreover the number of triangles is small: sprites are inherently low-poly because the surface is required to be a rectangle composed of 2 triangles to work. So I propose NUKING USE_VERTEX_SPRITE/vertexSprite_vp.glsl and just adding the vertexes from the CPU. Essentially, reverting b10e8c2.

Can't they just be converted to use vertexSimple_vp and a model matrix be calculated accordingly, so they're always facing the viewer?

[slipher]

Can't they just be converted to use vertexSimple_vp and a model matrix be calculated accordingly, so they're always facing the viewer?

Yeah, probably possible. One thing that could be wrong with such an approach is the light grid. But this stuff only really has to work with BSP surfaces, where the light grid (I think) can't be used. If it were used the errors would be small in most cases. It would probably fine as long as there isn't anything else that depends on having a correct world position.

The advantage would be you keep using the VBO. The disadvantage would be you always have to send every sprite (2 triangles) as a separate draw call since the matrices are different.

[VReaperV]

The advantage would be you keep using the VBO. The disadvantage would be you always have to send every sprite (2 triangles) as a separate draw call since the matrices are different.

We could put all matrices into a buffer, however it won't work on ancient hardware that doesn't support that.

[slipher]

Fixed by #1281.