lavaalpha issues in ne_ruins (invisible lava)

[Placo]

In the "Altar of Storms" mod:
https://www.quaddicted.com/reviews/ne_ruins.html

At some point during the second map the lava rises and floods the lower part of the level. It is invisible in vkQuake.

I noticed this bug just as vkQuake was archived nearly two years ago so I couldn't create an issue here but then I was reminded of this bug today.
The problem appeared around the time Temx reworked the engines' alphasorting, the mod was working fine in vkQuake in older versions.

I have tried disabling r_alphasort but it doesn't change the problem.
I noticed that r_wateralpha is at 0.7 and r_lavaalpha is at 0.
Changing either r_wateralpha or r_lavaalpha to 1 makes the lava appear, but any other value (between ==0 and <1) the lava is invisible.
I don't know if the issue is caused because the lava is invisible at the start of the map or what (as it rises during the level).

I use vkQuake in Win10 with a geforce980

In vkQuake:

In Ironwail for comparison:

A save to the spot with the problem:
save.zip

[j4reporting]

It seems that the "lava" in this map is a bit different from the usual lava (e.g.. id1/end ), as it is always rendered with r_lavaalpha 1 in ironwail and quakespasm. The value for r_wateralpha of 0.6 is not applied here ( r_lavaalpha "0" ).

[j4reporting]

first bad commit is 810725b
"Separate worldmodel water by transparency status"

It looks like only lava parallel to the floor is rendered incorrectly.
I think this map is using entities for lava with alpha 1.0 ( r_drawentities 0 ) will remove all lava

r_lavaalpha 1:

r_lavaalpha <1:

r_lavaalpha 1:

r_lavaalpha <0:

[vsonnier]

first bad commit is 810725b
"Separate worldmodel water by transparency status"

Thanks to have tracked down this. I may be worth reverting this "optimization" if the change is actually not too big. I'll look into when I have more time.

[vsonnier]

Thanks to have tracked down this. I may be worth reverting this "optimization" if the change is actually not too big. I'll look into when I have more time.

I started playing ne_ruins, and (by chance ?) with my usual transparency settings set in autoexec.cfg:

r_novis 0
r_wateralpha 0.8
r_slimealpha 0.85
r_lavaalpha 1.0
r_telealpha 1.0

I see no problem with lava so far:

so setting r_lavaalpha 1.0 is indeed the right thing to do for this map, which coincidently is the logical setting for lava IMHO.

There are too many trenparency combinations vs. rendering and in reality I'm not competent to try to change this without breaking anything else, so I'm closing this.

[j4reporting]

found another map with this issue. rvj_inky from Re:Mobilize Jam 2: Re:Visions .
It's basically map dm1 in a horror experiance similar to 'a day like no other'.
Here the water/slime, where usually lava is, near the steps in the mirror world are not rendered with r_wateralpha < 1 settings. As well as the exit portal at the end.

it seems map is not water-vised, but vkquake will render water/slime as black and the portal is not rendered at all.
EDIT: map source is unfiortunately not included in the pak file.

[j4reporting]

the common theme here seems to be liquid surfaces with alpha value of 1.0 in bsp are not rendered correctly if cvar r_wateralpha is < 1 ( assuming r_{tele,slime,lava}alpha 0 ).
Inky's map has a temple section with a pool which also changes from water to blood. This is rendered correctly, because both entities have alpha < 1.

All teleporters, even the two teleporters in the standard part of DM1, ( entities have alpha 1 ) and the water/slime pool in the mirror world ( also alpha 1 ) are affected by this bug.

[j4reporting]

Do we know what kind of artifacts this commit 810725b is supposed to fix?

There are too many trenparency combinations vs. rendering and in reality I'm not competent to try to change this without breaking anything else

It can be reverted w/o breaking anything else. Until we figure out what's missing.

This commit has some more issues or unexpected behaviour:
default settings with r_wateralpha 0.8

load savegame and suddenly the water surface of the water/slime pool is rendered correctly ( opaque = alpha 1 )
changing r_wateralpha to a value < 1 breaks the rendering again.
This does not work with the slime surface, it's always rendered incorrectly with r_wateralpha < 1, probably because its original starting position is somewhere else and replaces the watersurface.

rvj_savegames.zip

there is another location in this map, pond in the forest, with the same issue.

[vsonnier]

Oh no ! anyway.

@j4reporting Thanks for your previous investigation.
I have pushed the branch vso_revert_trensparent_water_optimization to test the revert.

Turns out there was one bug in that commit smashing the stack, luckily reported by MSVC in debug : the erroneous call to R_CalcSpecialsAndTextures in r_brush.c.

This is ok so far with as you said with rvj_inky with your provided savegames.

Load savegame and suddenly the water surface of the water/slime pool is rendered correctly ( opaque = alpha 1 )
changing r_wateralpha to a value < 1 breaks the rendering again.

This behaves similarily with this branch, is a sense that switching back and forth between r_wateralpha = 1.0 and < 1.0 is actually not symetric. I think this resolves around the fact that reading the r_wateralpha and such from the level is done once.

For instance, if we suppose the level has a built-in r_wateralpha 0.6:

• Start : Water is transparent with 0.6 => OK.
• Change r_wateralpha 0.8 manually => applied
• Change back to r_wateralpha 1.0 supposely meaning level default => opaque water, level r_wateralpha of level not re-read
• Change r_wateralpha 0.2 manually => applied
  ... and so on.

So, supposing you don't want to force tranparency in a level where there is already one specified, it is better to stick with the default transparency values at startup (meaning : apply the ones of the level)

I changed my autoexec.cfg accordingly, like this:

r_wateralpha 1.0
r_slimealpha 0
r_lavaalpha 0
r_telealpha 0
alias al_watertoggle_on "echo Water tranparency on ; r_wateralpha 0.7 ; r_slimealpha 0.85 ; r_lavaalpha 1.0 ; r_telealpha 1.0 ; bind w al_watertoggle_off"
alias al_watertoggle_off "echo Water tranparency default ; r_wateralpha 1.0; r_slimealpha 0.0 ; r_lavaalpha 0.0 ; r_telealpha 0.0 ; bind w al_watertoggle_on"
bind w al_watertoggle_on  

I bounded w to switch from default to forced transparency, but as said above this is not symetric and if you force it once, the original setting of the level is lost.

[j4reporting]

is one of the maps included in sm230 affected, too?

This behaves similarily with this branch, is a sense that switching back and forth between r_wateralpha = 1.0 and < 1.0 is actually not symetric. I think this resolves around the fact that reading the r_wateralpha and such from the level is done once.

Some maps will set _wateralpha in worldspawn, so it's ok to override with r_wateralpha but if an entity has a specific alpha value assigned in the map, then it should not be affected by any global change.

Inky was so kind to send me the map source file and the water/slime pool has an alpha setting of 1.0 for water and slime.
Changing the cvar r_wateralpha or r_slimealpha should not affect this edicts, similar to the water/blood pool in the forest temple. Those edicts have alpha settings of <1.0 and are not affected by any change of the global r_*alpha settings.

// entity 1167
{
"classname" "func_togglevisiblewall"
"alpha" "1"
"spawnflags" "3"
"targetname" "vw_grim"
"_mirrorinside" "1"
"_tb_layer" "38"
// brush 0
{
( 416 192 16 ) ( 416 193 16 ) ( 416 192 17 ) skip [ 0 -1 0 -48 ] [ 0 0 -1 -48 ] 0 1 1
( 432 64 16 ) ( 432 64 17 ) ( 433 64 16 ) skip [ 1 0 0 16 ] [ 0 0 -1 -48 ] 0 1 1
( 432 192 16 ) ( 433 192 16 ) ( 432 193 16 ) skip [ -1 0 0 -16 ] [ 0 -1 0 -48 ] 0 1 1
( 464 224 128 ) ( 464 225 128 ) ( 465 224 128 ) *slime2 [ 1 0 0 16 ] [ 0 -1 0 -48 ] 0 1 1
( 464 368 32 ) ( 465 368 32 ) ( 464 368 33 ) skip [ -1 0 0 -16 ] [ 0 0 -1 -48 ] 0 1 1
( 480 224 32 ) ( 480 224 33 ) ( 480 225 32 ) skip [ 0 1 0 48 ] [ 0 0 -1 -48 ] 0 1 1
}
}
// entity 1168
{
"classname" "func_togglevisiblewall"
"alpha" "1"
"targetname" "vw_grim"
"spawnflags" "2"
"_mirrorinside" "1"
"_tb_layer" "38"
// brush 0
{
( 416 256 16 ) ( 416 257 16 ) ( 416 256 17 ) skip [ 0 -1 0 -48 ] [ 0 0 -1 16 ] 0 1 1
( 416 64 16 ) ( 416 64 17 ) ( 417 64 16 ) skip [ 1 0 0 32 ] [ 0 0 -1 16 ] 0 1 1
( 416 256 16 ) ( 417 256 16 ) ( 416 257 16 ) skip [ -1 0 0 -32 ] [ 0 -1 0 -48 ] 0 1 1
( 448 288 128 ) ( 448 289 128 ) ( 449 288 128 ) *water2 [ 1 0 0 32 ] [ 0 -1 0 -48 ] 0 1 1
( 448 368 32 ) ( 449 368 32 ) ( 448 368 33 ) skip [ -1 0 0 -32 ] [ 0 0 -1 16 ] 0 1 1
( 480 288 32 ) ( 480 288 33 ) ( 480 289 32 ) skip [ 0 1 0 48 ] [ 0 0 -1 16 ] 0 1 1
}
}

func_togglevisiblewall will change the visibility. Water entity becomes disabled and the slime entity is going to be rendered instead. Both have alpha 1

spawnflags   func_togglevisiblewall
	1 : "Start Off" : 0
	2 : "Non-solid" : 0
	4 : "Keep visible" : 0

[j4reporting]

just noticed that branch vso_revert_trensparent_water_optimization introduces other issues with rvj_inky and probably other maps.

r_wateralpha 1 r_{lava,slime,tele}alpha 0

missing brushes, some teleporters are 'empty' , it get's even worse with r_indirect 0

rvj_inky_door.mp4

[vsonnier]

Oh no ! anyway.
is one of the maps included in sm230 affected, too?

Sorry, that was just a joke :) So kind of people to provide more test case for us.

just noticed that branch vso_revert_trensparent_water_optimization introduces other issues with rvj_inky and probably other maps.

I see the same in rvj_inky indeed.

Was it OK with your version of the revert ? If yes, the only difference is the very erroneous usage of R_CalcSpecialsAndTextures in r_brush.c that I removed.

Edit : this is likely because of my 'hardening' of Mod_CalcSpecialsAndTextures in cf9054c I probably misunderstood something in this function...

[vsonnier]

Right, I've re-pushed vso_revert_trensparent_water_optimization with a fix in Mod_CalcSpecialsAndTextures :

I was trying to mark psurf->texinfo->tex_idx = -1 once psurf was taken into account, but psurf->texinfo was aliased in other psurf so I was actually losing track of the texture for all the the other psurf sharing the same texinfo. Making things randomly texture-less in the world.

I kept the out-of bound check just in case, triggering Host_Error.

[j4reporting]

I just reverted cf9054c and it seems to be fine. ;)
will retry with new commit

Master renders correctly if I remove lines 1157 and 1158 in

vkQuake/Quake/r_world.c

Lines 1153 to 1160 in 1d1e80c

	int lastlightmap = -2;
	const float alpha = GL_WaterAlphaForEntitySurface (ent, t->texturechains[chain]);
	const qboolean alpha_blend = alpha < 1.0f;
	if (((opaque_only && alpha_blend) || (transparent_only && !alpha_blend)) && (!r_lightmap_cheatsafe || !indirect))
	continue;
	gltexture_t *gl_texture = t->warpimage;

not sure what the cost is here.

[vsonnier]

Master renders correctly if I remove lines 1157 and 1158 in

vkQuake/Quake/r_world.c

 if (((opaque_only && alpha_blend) || (transparent_only && !alpha_blend)) && (!r_lightmap_cheatsafe || !indirect)) 
 	continue; 

not sure what the cost is here.

I replayed ne_ruins with it and indeed it looks perfect with the defaults r_wateralpha 1 ; r_{lava,slime,tele}alpha 0

No apparent performance impact on my side, on my not-so recent gaming laptop.
In any case solving a graphic bug is more important here.

[j4reporting]

No apparent performance impact on my side, on my not-so recent gaming laptop. In any case solving a graphic bug is more important here.

I'd guess this are the 'slight performance improvements' mentioned by temx in his commit.

I wonder if this is a general issue with edicts with a fixed alpha =1 value? They seem to be used rarely (only two maps so far) that we are aware of. Detect if such edict exist and disable the shortcut in line 1157 + 1158 or handle the alpha value as 0.99 internally?

[vsonnier]

Detect if such edict exist and disable the shortcut in line 1157 + 1158

I would prefer an explicit test localized here than altering the original alpha value.

Which test though ? only try to shortcut if alpha_blend == true ?

[j4reporting]

Detect if such edict exist and disable the shortcut in line 1157 + 1158

I would prefer an explicit test localized here than altering the original alpha value.

sure, altering the alpha value just as an experiment / idea.

Which test though ? only try to shortcut if alpha_blend == true ?

when edict are read from map file? If such an edict with alpha = 1.0 is detected disable the shortcut for the map.

[vsonnier]

when edict are read from map file? If such an edict with alpha = 1.0 is detected disable the shortcut for the map.

I'll be honest, this is awful no thanks :). I would disable the piece of code entirely (keeping it between #if 0 .. #endif), waiting for a better solution later, with some comment to explain the problem.

[j4reporting]

I'll be honest, this is awful no thanks :). I would disable the piece of code entirely (keeping it between #if 0 .. #endif), waiting for a better solution later, with some comment to explain the problem.

I did not claim that it would be an elegant workaround ;) 99.9% of the maps do work fine with this code in place,

I grepped through some map source files, and if alpha is used then mostly with values <1
Inky's map and ne_ruins are so far the only maps with alpha values of 1.

ne_ruins.bsp has strings like

{
"classname" "func_mover_liquid"
"alpha" "1.0"

We know that both maps are rendered correctly in an opaque world ( ne_ruins needs r_lavaalpha 1 to render correctly, because map sets _wateralpha 0.6 in worldspawn).
In a transparent world, these surfaces are not selected for rendering at all. This is the (transparent_only && !alpha_blend) part in line 1157.

The code assumes that ALL liquid surfaces are transparent in a transparent world with alpha_blend active.

[vsonnier]

Right, I admit I'm lost. Please @j4reporting provide a PR if you have an idea because you definitly have one it seems :)

[j4reporting]

sorry, no PR atm. I only have limited understanding of the code and it's probably wrong anyway ;)
It's probably best to disable the shortcut for now. I guess you would not want to disable the shortcut code for specific map names only ( two known maps so far). :p

[vsonnier]

I guess you would not want to disable the shortcut code for specific map names only ( two known maps so far).

No indeed not, even if it is only 2 maps (so far) since there is no (apparent) drawback to disable it.

In the meantime, we can live with r_drawwater_fast 0 by default:

In ne_ruins:

• r_drawwater_fast 0:

• r_drawwater_fast 1:

Using either liquid defaults (r_wateralpha 1, r_{lava,slime,tele}alpha 0) or forced alpha (r_wateralpha 0.7, r_{lava,slime,tele}alpha 1.0) give the same picture.