Fix lighting for movable BSP entities

[slipher]

BSP model surfaces were wrongly flagged with the bspSurface bit which only works for fixed-location world surfaces. This flag was causing the code to skip setting up the model matrix for the lightMapping shader, meaning normals and dynamic light positions were not correct.

So stop flagging BSP models as bspSurface. Also skip the other things in R_AddWorldSurface, besides R_AddDrawSurf, because they are useless or redundant.

Fixes Unvanquished/Unvanquished#3474.

[illwieckz]

This has the unwanted side effect of forcing all BSP models to be rendered with the lightgrid instead of their lightmap.

Actually, I want to implement that feature of being able to render some BSP models with the lightgrid (it's useful for movers like doors or trains, to not move with their shadows), but we better not enforce that.

BSP models are BSP surfaces and they are not wrongly flagged with the bspSurface bit, but we need more metadata to handle the various use cases:

• non-model bsp surface
• lightmapped model bsp surface
• lightgrid model bsp surface

As a start we may only focus on those two uses cases (to only fix the bug you found) :

• non-model bsp surface
• model bsp surface

Either we add more booleans, either we turn bspSurface into a bit field.

[illwieckz]

Such bitfield can have three values (better names can be found):

• BSP_SURFACE (all BSP surfaces would get this bit)
• BSP_MODEL (only BSP models would get this bit)
• BSP_LIGHTMAP (all BSP surfaces would get this bit for a start, configurable in the future)

To fix the bug you found we would check that BSP_SURFACE is set but BSP_MODEL is not set, and since BSP_LIGHTMAP would always be set the lightmapping would still be correct.

And in the future we would implement a code path to not set BSP_LIGHTMAP on some models (likely a map entity option) to disable the lightmap on a given model and enforce the lightgrid for it.

[illwieckz]

The Vega map has many doors and can be used to test this. Among those doors is a vertical door with a known visible shadow moving with the door (that's the kind of door I would like to have an option to force the lightgrid on it).

[slipher]

This has the unwanted side effect of forcing all BSP models to be rendered with the lightgrid instead of their lightmap.

I'm aware of that. I had written it in the commit message, but forgot to push the latest version.

Lighting the models with the light grid seems like a correct default to me, since in general they are intended to move, meaning the fixed lightmap is wrong. Stuff is usually a BSP model rather than part of the main world model because it needs to move. Maybe there are some cases that don't move, but the gamelogic would have to flag those.

[illwieckz]

Lighting the models with the light grid seems like a correct default to me

It is a good default, but no maps were built for that.

since in general they are intended to move, meaning the fixed lightmap is wrong.

Indeed, but grid lighting looks less good and for various use cases, like doors opening by retracting inside walls left and right, lightmap is good enough. And I don't know if it's a bug in gridlight code, but lightgrid looks far worse on them.

For some reasons, lightgrid doors can look very different according from the side we look at them:

Another door:

But the main argument is that all maps were done with the fact those models were lit with the lightmap.

Here is a good example, the original metro map.

The train is lit with lightmap, the mapper created a special room to store the train at light backing time to lit it evenly. The mapper added a fog in the tunnel which blends with the train and emulate the fact the train comes to light. But the mapper also did a workaround for lighting the entrance of the tunnel by adding a point light at the entrance of it.

When we lit the train with the lightgrid, we can see the train being lit by the workaround light:

Of course if this map was made brand new today, we would not use such point light workaround and we would use light grid on the model, using the actual light grid to actually render the fact the train comes from obscurity to light. The metro map is one of the reasons I want the feature of lighting BSP models with the grid light, and in my WIP revamp of the map, I deleted that workaround point light.

But I consider too bold to change the default.

[slipher]

For some reasons, lightgrid doors can look very different according from the side we look at them:

Yeah... the light grid equations used in our code are totally idiotic and wrong. I've been reading the TraceGrid function from q3map2 to understand how the light grid data is intended to be used. I'm working on rewriting the code right now, so let's see if I can get something that comes out closer to the lightmap result.