Multiply with var_Color in lightMapping after u_AlphaThreshold check

[VReaperV]

Fixes an issue creating a void effect, where vertex + color/diffuse map produces different results in depth pre-pass generic and lightMapping shaders, due to different check/multiplication order.

Fixes #119. I've also tried moving the multiplication before the check in generic shader, but it ended up making surfaces disappear based on distance to them.

[slipher]

So what is "alphagen vertex" used for?

Also why does q3map2 even write values other than 1 to the alpha of the vertex color? Seems weird

[VReaperV]

I suppose it's still used for blending.

[illwieckz]

Also why does q3map2 even write values other than 1 to the alpha of the vertex color? Seems weird

It is used for terrain blending, for example in a single shader you have a rock and a sand stage, and one of the stage receives its alpha channel from the vertex, this makes possible to blend two terrains per surface.

[VReaperV]

Also why does q3map2 even write values other than 1 to the alpha of the vertex color? Seems weird

It is used for terrain blending, for example in a single shader you have a rock and a sand stage, and one of the stage receives its alpha channel from the vertex, this makes possible to blend two terrains per surface.

Do we have an example of such a shader?

[illwieckz]

The garden in station15 (grass/mud), the outdoor of thunder (rock/sand).

[illwieckz]

It modifies the way the grass/mud floor is rendered in the garden:

Before:

After:

Now it's possible that is the map that is wrong. I had to edit it to workaround a q3map2 bug, but I remember I struggled to get the effect in the first screenshot, as I didn't seen much correlation between what I did in editor and the result I was getting.

[illwieckz]

This is how it looks in editor:

There is no place where the grass is 0% (mud 100%).

So maybe this branch is right and the map source is wrong.

[illwieckz]

If I set some 0%, I see the grass surfacing where it is 0%:

[illwieckz]

If I add more grass (less mud):

I get more grass:

With this branch, what I do in editor is what I get in game.

[illwieckz]

LGTM.

[slipher]

There was a discussion in IRC in which SomaZ said that actually the generic shader is the "correct" one, in terms of matching q3 behavior.

[SomaZ]

Changing vertex alpha multiplication to be performed after the alpha discard is wrong (in terms of compatibility). Changing it to be performed before the discard in the other shaders like the generic shader would be the correct fix in terms of compatibility. This should also fix the incorrect void effect illwieckz encountered on station15 with the trees.

[VReaperV]

There was a discussion in IRC in which SomaZ said that actually the generic shader is the "correct" one, in terms of matching q3 behavior.

The generic shader actually does multiplication after discard.

[VReaperV]

Changing vertex alpha multiplication to be performed after the alpha discard is wrong (in terms of compatibility). Changing it to be performed before the discard in the other shaders like the generic shader would be the correct fix in terms of compatibility. This should also fix the incorrect void effect illwieckz encountered on station15 with the trees.

I tried that, and the small plants on the ground in station15 in the same area started disappearing based on distance. Although perhaps that is another map bug that was trying to workaround an engine issue.

[slipher]

I tried that, and the small plants on the ground in station15 in the same area started disappearing based on distance. Although perhaps that is another map bug that was trying to workaround an engine issue.

Probably it's the same issue as the trees, with the alpha volumes, which are intended to affect only the ground, doing collateral damage to the plant models? If there are a lot of bad alpha volumes, maybe it would be easier to put alphaGen identity in the plant shaders rather than moving all of them.

Changes like this which break the current version of our assets can be targeted to the for-0.56.0/sync branch.

[illwieckz]

Actually multiplying before testing makes sense if one wants to combine alphaGen vertex with alphaFunc GE128, which is what does the textures/station15_custom/ter_mudmoss1 shader.

This may look weird, but it allows to do alpha blending with a strong cutoff: instead of linearly blending from moss to mud, only values above 50% makes mud appear, and everything elle displays full moss.

The fact I didn't felt something was meaningful between what was done in the editor and what was gotten in game was because I did not notice that cut-off. Now that I know about it, what I get in game is what I do in editor while taking such cut-off in consideration (only using alpha brushes higher than 50%).

[illwieckz]

I tried that, and the small plants on the ground in station15 in the same area started disappearing based on distance. Although perhaps that is another map bug that was trying to workaround an engine issue.

Probably it's the same issue as the trees, with the alpha volumes, which are intended to affect only the ground, doing collateral damage to the plant models.

Yes, it's likely the same problem, as we see on above screenshot some plants are in some alpha volumes. It can be worked around by having smaller alpha volume that only hits the brushes intersections to mark them. Doing a large alpha brush is just a lazy way to mark all brushes intersections from a whole area.

Also for the tree, the alpha brush can just be made less high, but that would not work for plants.

Actually I'm happy we start grasping what's going on after all those years.

This now looks wrong.

[VReaperV]

I've added a commit that reverts the change to lightMapping and changes the generic shader instead, but now the plant isn't rendered. Maybe you could look at the map if it's a map bug.

[slipher]

LGTM for putting on the 0.56 branch.

[VReaperV]

(rebased & squashed, +re-targeted to for-0.56.0/sync)

[VReaperV]

Actually, I tried this with rebasing on master, and all the plants are now rendered correctly, probably because something was fixed on master or how it worked changed. So I think it's fine to merge this into master.

[slipher]

I do get a partially missing plant when cherry-picking this onto master. The one in the upper right.

Before

After

[illwieckz]

Now I'm a bit lost… As far as I understood this branch is not anymore about preventing those models to disappear but to fix the blending math. The model being disappearing should be fixed in the material itself.

So if the current patch is doing the contrary to what was initially proposed, i.e. keeping the code that was making models disappear, and modifying the remaining code to do the same, then it looks good to me.

I have a branch for the station15 map that moves the alpha brushes out of those models so the models don't disappear, and I remember @SomaZ made some suggestions to modify the materials to prevent the bug to occur even if they are within alpha brushes (I will also test this).

The disappearance of those models can be fixed in two ways (one verified, one to be verified) in the map while keeping correct the blending math in engine.

[illwieckz]

This doesn't seem to be rebased yet on for-0.56.0/sync (I tried to compile it with Unvanquished being on for-0.56.0/sync.

[VReaperV]

Hmm, I tried this again and now I just get the plant above partially disappearing, and the one in #119 completely invisible. So the result I got earlier might be a shader cache issue.

So if the current patch is doing the contrary to what was initially proposed, i.e. keeping the code that was making models disappear, and modifying the remaining code to do the same, then it looks good to me.

It is, yeah.

I have a branch for the station15 map that moves the alpha brushes out of those models so the models don't disappear, and I remember @SomaZ made some suggestions to modify the materials to prevent the bug to occur even if they are within alpha brushes (I will also test this).

Which branch is that?

[SomaZ]

Doing vertex color multiply before discard check is correct in terms of compatibility. Should fix void effect on surfaces using different shaders where it was handled correctly before. Main issue with the trees in this case is, that there's an alpha brush that sets some of the trees vertices alpha. An alphaGen identity in the trees shader should prevent the usage of the incorrectly set vertex alpha values.

[VReaperV]

(rebased on for-0.56.0/sync)

[VReaperV]

Doing vertex color multiply before discard check is correct in terms of compatibility. Should fix void effect on surfaces using different shaders where it was handled correctly before. Main issue with the trees in this case is, that there's an alpha brush that sets some of the trees vertices alpha. An alphaGen identity in the trees shader should prevent the usage of the incorrectly set vertex alpha values.

Changing the plants and trees to use alphaGen identity does, indeed, work.