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

Introduce r_lightMode, implement vertex lighting and use it when needed. #992

Merged
merged 2 commits into from
Jan 10, 2024

Conversation

illwieckz
Copy link
Member

@illwieckz illwieckz commented Dec 12, 2023

Historically we had the r_vertexLighting cvar, but we didn't have vertexLighting, in fact we used it as reverse cvar for enabling light mapping:

  • r_vertexLighting 0: enable light mapping (default)
  • r_vertexLighting 1: disable light mapping, fallback on grid lighting

This replaces it with r_lightMode, with possible values:

  • r_lightMode 0: fullbright
  • r_lightMode 1: vertex lighting
  • r_lightMode 2: grid lighting
  • r_lightMode 3: light mapping (default)

The only r_lightMode values expected to be set by players are 1 (vertex lighting) and 3 (light mapping), especially since others can be a ways to cheat. To use other light modes one should set r_cheatLightModes to on.

Vertex lighting does not do less computation than light mapping, but skips the uploading of lightmaps and then save on GPU memory size, it is the only reason why it's not a bad idea to provide it to users.

Light grid is not faster than light mapping, there are more GLSL code to execute, it may uses less GPU memory though, but on 20 years old hardware I got less fps last time I compared. Grid lighting is cheaty because the light grid skips the projected shadow.

Fullbright is cheaty.

The cheat light modes are meant for debugging purposes, to give the developer/mapper the ability to see the map with those various lighting techniques.

Currently the engine uses light mapping on world surfaces and grid lighting on game models (players, buildables, weapons…), that's why we have both light mapping and grid lighting implemented. We also used grid lighting on world surfaces when there is no lightmap for the surface. #990 was an example of bug occurring when there is no fallback for world surfaces without lightmap.

But it is expected world surfaces without lightmap don't use grid lighting but vertex lighting. Why? Because the q3map2 map compiler has an option to skip some lightmaps when the color is very similar to vertex colors. So basically, with that option, if the light is consistent, the surface doesn't get a lightmap, and if the light is not consistent (for example it has a projected shadow), it gets a lightmap. A game like Wolf:ET made strong usage of that technique and some maps had like only half of surfaces having a lightmap. On maps using such optimization, the light guessed from the lightgrid is close to the lightmap one but not equal, it makes the map look checkered, since surfaces lit with lightgrid don't match the surfaces lit with lightmap. We should then use vertex lighting on world surfaces without lightmaps.

Since I needed to implement vertex lighting for those surfaces, it was easy to add this light mode to r_lightMode options.

…e lighting selection, fix bugs

- Introduce r_lighting cvar with values:

0: fullbright
1: vertex lighting
2: grid lighting
3: light mapping

The default value is 3 (light mapping), users may also use 1 (vertex
lighting).

All other values use the light mapping code but with different inputs,
so none of the others are expected to be faster than light mapping.

Values 0 (fullbright) and 2 (grid lighting) may give unfair advantages
without satisfying a need for performance, and are considered cheats.

The only thing grid lighting or vertex lighting can do is to save GPU
memory by not uploading light maps, but the code itself is not faster,
and is even a bit slower with grid lighting because there is more
computation to do.

- Implement fullbright lighting as a debug option.
- Implement vertex lighting for both in-game usage when needed and as a debug option.
- Use vertex lighting for bsp surfaces without lightmap.
- Deeply rewrite the code to make it matching more the description of the need,
  make it less implicit, and track down bugs.
- Limit usage of Half Lambert Lighting and Wrap Around Lighting for models.
- Wrap Around Lighting uses the value set in material.
- Properly enable Rim Lighting.
- Define more GLSL macro conflicts to help the engine skip unused combinations.
- Recognize light-styles lightmaps and process all light-styles
  lightmaps with Render_generic to avoid dynamic lights.
- Properly set implicitLightmap on textures without materials.
- Do not render lightmaps on collapsed stages without lightmaps.
- Make fullbright and grid lighting for world rendering cheats.

The engine may still use fullbright rendering or light grid rendering
when needed (models use light grid) or as a fallback.

renderer: correct light style implementation
@illwieckz illwieckz added A-Renderer T-Feature-Request Proposed new feature labels Dec 12, 2023
@illwieckz illwieckz marked this pull request as draft December 12, 2023 05:10
@illwieckz
Copy link
Member Author

I may turn the cvar into a modern one and use a range too, and the final commits may differ, that's why I currently marked the PR as draft, but it should work.

@illwieckz
Copy link
Member Author

illwieckz commented Dec 12, 2023

Some things:

I noticed that with fullbright the game models still use grid lighting. I'll look at it later. I consider this a bug, though totally minor since it only affects a debug mode.

I also noticed I forgot to disable deluxe mapping when vertex lighting is enabled.

I want to use grid lighting for world movers, if not enforced, at least optional for the mapper. It would prevent the movers to move with their baked shadow, but get the light color and intensity from their position. That may be another PR anyway.

If we want to keep grid lighting and vertex ligthing not a cheat, we may do a r_fullbright cvar instead so r_lighting can chose between vertex lighting, grid lighting, light mapping.

@illwieckz
Copy link
Member Author

I finally did a very deep revamp, to make the code more obvious, and relying less on hacks.

Especially, I had to add a macro to avoid guessing the information out of other macros in GLSL, this means I had to increase by 1 the amount of macros a single shader can received, but since I also did other rewrites to make sure some macro can exclude themselves, the amount of compiled shaders are exactly the same in the end.

All the uses case I wanted to take care about are now working.

I still see one bug with a Wolf:ET map but it is not introduced by this code and may even require to dive deeper in Wolf:ET specificity so it's out of topic twice.

@illwieckz
Copy link
Member Author

illwieckz commented Dec 13, 2023

I believe vertex lighting should not be faster than light mapping because the implementation is simply to blend vertex light color while using a white light map, meaning the light map code runs in all cases. All it would do would be to save GPU memory by not uploading light maps. To get a faster implementation one would need to write more GLSL code and more C++ plumbing, which is likely not worth the effort. In all cases the base line is light map and that's not bad as we should focus on that.

@illwieckz
Copy link
Member Author

illwieckz commented Dec 14, 2023

While doing that work I stumbled on many other bugs (I fixed) like:

  • a variable for the rim lighting feature only meant to be used for models was only set for surfaces of the map;
  • while we have both enabled half lambert and wrap around lighting for models, only one of them was used at once;
  • lightgrid used to guess light direction to emulate the deluxemap for models was always set when a deluxemap was missing, even if there was no computation to do;
  • etc.

@illwieckz
Copy link
Member Author

illwieckz commented Dec 14, 2023

In fact with latest iteration it even fixes the light rendering of Wolf:ET maps like Oasis (something I was aware for 8 years!) and foliage lighting of Wolf:ET maps like Oasis and Goldrush. I'm now unaware of any remaining map lighting bug.

@illwieckz illwieckz force-pushed the illwieckz/lighting branch 4 times, most recently from 096bc0c to 7c1f84d Compare December 14, 2023 16:03
@illwieckz
Copy link
Member Author

illwieckz commented Dec 14, 2023

So, here are some before/after comparisons, with those maps:

game map viewpos
tremulous rusty -1672 -936 88 -55 3
quake3 school 205 1140 5 36 8
unvanquished grangermaze -1705 2600 216 135 51
wolfet oasis 2219 2861 -31 93 13
wolfet goldrush 2050 -533 -115 -116 10
smokinguns elpaso 566 725 99 -125 7
tremulous karith 2080 4484 -416 120 0
unvanquished chasm -2627 1622 272 -38 0
unvanquished hangar28 -1286 740 -765 -28 23
unvanquished plat23 2240 2100 230 42 16
unvanquished forlorn 2346 1124 442 94 30

rusty

0.54.1, missing light fallback on models without lightmap (regression):

unvanquished_2023-12-14_214931_000

master, grid light used as fallback (revert of the regression):

unvanquished_2023-12-14_220745_000

this branch, vertex lighting used as fallback (what should be done):

unvanquished_2023-12-14_221427_000

school

0.54.1, missing light fallback on models without lightmap (regression):

unvanquished_2023-12-14_220018_000

master, grid light used as fallback (revert of the regression), but with a default lightgrid because this map doesn't have one:
unvanquished_2023-12-14_220818_000

this branch, vertex lighting used as fallback (what should be done):

unvanquished_2023-12-14_221451_000

grangermaze

master, regression on nolightmap lights being shaded by mistake:

unvanquished_2023-12-14_220850_000

this branch, what should be done:

unvanquished_2023-12-14_221511_000

@illwieckz
Copy link
Member Author

oasis

master, grid light used as fallback (what the engine did before):

unvanquished_2023-12-14_220911_000

this branch, vertex lighting used as fallback (what should be done):

unvanquished_2023-12-14_221546_000

goldrush

master, grid light used as fallback (what the engine did before):

unvanquished_2023-12-14_220933_000

this branch, vertex lighting used as fallback (what should be done):

unvanquished_2023-12-14_221607_000

elpaso

master, as expected:

unvanquished_2023-12-14_222835_000

this branch, no regression:

unvanquished_2023-12-14_222910_000

karith

master, as expected:

unvanquished_2023-12-14_220948_000

this branch, no regression:

unvanquished_2023-12-14_221629_000

@illwieckz
Copy link
Member Author

illwieckz commented Dec 14, 2023

chasm

master, as expected:

unvanquished_2023-12-14_221013_000

this branch, no regression:

unvanquished_2023-12-14_221645_000

hangar28

master, as expected:

unvanquished_2023-12-14_221110_000

this branch, no regression, this scene is known to break when moving out of the room with noclip if we change the surface properties instead of using temporary variables to set vertex lighting fallback:

unvanquished_2023-12-14_221719_000

plat23

master:

unvanquished_2023-12-14_221154_000

this branch, models lighting may have subtle differences since features that were wrongly disabled are now enabled:

unvanquished_2023-12-14_221809_000

forlorn

master:

unvanquished_2023-12-14_221216_000

this branch, models lighting may have subtle differences since features that were wrongly disabled are now enabled:

unvanquished_2023-12-14_221929_000

Actually, the armoury screen is interesting on this one.

@illwieckz illwieckz marked this pull request as ready for review December 17, 2023 00:26
@illwieckz
Copy link
Member Author

This is now ready for review.

I played all my recent games with that branch, and the only things I spotted were fixes, even unexpected ones.

@illwieckz illwieckz added T-Bug T-Improvement Improvement for an existing feature labels Dec 17, 2023
Copy link
Member

@slipher slipher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will review more later.

Can you explain the concept of vertex lighting more? I don't understand where the lighting information comes from in that case.

src/engine/renderer/glsl_source/computeLight_fp.glsl Outdated Show resolved Hide resolved
src/engine/renderer/tr_init.cpp Outdated Show resolved Hide resolved
src/engine/renderer/gl_shader.cpp Outdated Show resolved Hide resolved
@illwieckz
Copy link
Member Author

illwieckz commented Dec 17, 2023

Can you explain the concept of vertex lighting more? I don't understand where the lighting information comes from in that case.

In the BSP file, in the struct storing vertexes, there is an RGBA color field:

Vertexes

The vertexes lump stores lists of vertices used to describe faces. There are a total of length / sizeof(vertex) records in the lump, where length is the size of the lump itself, as specified in the lump directory.

vertex  
float[3] position Vertex position.
float[2][2] texcoord Vertex texture coordinates. 0=surface, 1=lightmap.
float[3] normal Vertex normal.
ubyte[4] color Vertex color. RGBA.

Cf. http://www.mralligator.com/q3/#Vertexes

Those vertex colors are computed by q3map2 at the same time the light grid and the light maps are computed. It means we have 3 sources of light information for every BSP surface, two directly tied to each surface (lightmap, vertex color), one guessable from the nearby space (lightgrid).

The code modified in that PR doesn't process those colors, they are assumed to be already applied by the engine in a way the lightmapping GLSL code can blend with them if the correct operation is set.

The reason why I assume that is that we already have other features that require that underlying mechanism to be already working, for example terrain blending. This is the feature we can see in outdoor of Thunder map where rock texture blends with sand texture, or in garden in Station15 map, were grass blends with mud. This terrain blending uses vertex colors as well, more precisely the alpha channel of it, the engine is instructed to use an operation that would use such alpha channel to blend a lightmap stage over an already painted lightmap stage.

In case of vertex lighting, what I just do is then to set some color field to black and then zero, and tell the engine is has to use an operation to blend over colors. I assume that color field set in lightmapping C++ code do not carry the vertex color and is only used with some operations with vertex colors (here, probably an addition), hence the need for such color field to be zeroed to get the expected results. The alpha channel appears to also need to be zeroed (transparent) when the vertex lighting has to be applied on a transucent texture.

Those values (what operation to set, what color to set for the operation: black, transparent…) were guessed with intuition and some kind of trial & error process as I miss knowledge and understanding of some parts of the engine, but I knew vertex colors should already be applied by the engine and that all I had to do was to find the proper blend operation.

Vertex color blending was already implemented (otherwise many features would be broken), what I had to do was to tell the engine to blend textures over such vertex color when vertex lighting is enabled, which is like a 5 line patch.

The vertex colors are very likely interpolated somewhere in the engine, this is needed for the terrain blending, and we can see on school map screenshot above some emulation of projected shadow on some models using vertex colors, this can be done by interpolating colors of all we have to emulate a shadow is to darken the surface by interpolate light to dark colors between one point to another (and then, between edges).

Everything else in that PR is some very detailed decision code to be make all various lighting mode usables in a single scene, plus other rewrites that makes writing that decision code easier.

src/engine/renderer/tr_shade.cpp Outdated Show resolved Hide resolved
src/engine/renderer/tr_shade.cpp Outdated Show resolved Hide resolved
src/engine/renderer/tr_shade.cpp Outdated Show resolved Hide resolved
@illwieckz
Copy link
Member Author

illwieckz commented Jan 8, 2024

I can't also exclude there would be actual hacks in lightgrid code (either in q3map2 map compiler, either in engine), to artificially increase light from lightgrid to workaround the fact models get only one source of light without bouncing and other mechanisms.

@illwieckz
Copy link
Member Author

illwieckz commented Jan 8, 2024

If we want to give users alternate ways of lighting the map, we can give

  • light mapping
  • vertex lighting

Like Quake 3 and Tremulous did.

Vertex lighting should be no faster than light mapping: we still apply a stub lightmap and we can't avoid that without making the code much more complex, so the computation is as heavy, but it will save GPU memory by not uploading lightmaps.

Grid lighting uploads lightgrid instead of lightmap, this may use less GPU memory, but the lightgrid computation is heavier than the lightmap/vertex lighting one. I see no reason to provide a grid lighting option to users for performance purpose (even not counting the fact the compute is broken with overBrightBits).

@illwieckz
Copy link
Member Author

I did some tests with some maps, actually the benefit of lightgrid in overthehill is not that big.

It looks like the big difference affects newer maps like our official ones, and some others like rusty, maybe there is a q3map2 bug…

overthehill

viewpos -1113 298 159 57 5; lightmap, lightgrid, vertex light:

unvanquished_2024-01-08_114511_000

unvanquished_2024-01-08_114524_000

unvanquished_2024-01-08_114541_000

viewpos -470 631 56 105 -0; lightmap, lightgrid, vertex light:

unvanquished_2024-01-08_114627_000

unvanquished_2024-01-08_114638_000

unvanquished_2024-01-08_114650_000

viewpos 521 513 271 -136 15; lightmap, lightgrid, vertex light:

unvanquished_2024-01-08_114735_000

unvanquished_2024-01-08_114755_000

unvanquished_2024-01-08_114808_000

rusty

default view; lightmap, lightgrid, vertex light:

unvanquished_2024-01-08_121147_000

unvanquished_2024-01-08_121159_000

unvanquished_2024-01-08_121212_000

chasm

default view; lightmap with deluxemap, lightgrid, vertex light:

unvanquished_2024-01-08_120712_000

unvanquished_2024-01-08_120723_000

unvanquished_2024-01-08_120737_000

plat23

default view; lightmap with deluxemap, lightmap, lightgrid, vertex light:

unvanquished_2024-01-08_120848_000

unvanquished_2024-01-08_120904_000

unvanquished_2024-01-08_120919_000

unvanquished_2024-01-08_120940_000

@illwieckz
Copy link
Member Author

Enabling deluxemapping makes the map a bit brighter too, but not as strong as using light grid on recent maps.

Actually people can already get brighter maps with r_gamma so forbidding light grid is probably pointless anyway.

@illwieckz
Copy link
Member Author

The big problem of lightgrid is that it completely ignores projected shadows:

arachnid2

viewpos -697 -1144 586 -74 18; lightmap with deluxemap, lightmap, lightgrid, vertex light:

unvanquished_2024-01-08_122325_000

unvanquished_2024-01-08_122338_000

unvanquished_2024-01-08_122400_000

unvanquished_2024-01-08_122419_000

viewpos -548 -1644 337 -74 -2; lightmap with deluxemap, lightmap, lightgrid, vertex light:

unvanquished_2024-01-08_122508_000

unvanquished_2024-01-08_122523_000

unvanquished_2024-01-08_122544_000

unvanquished_2024-01-08_122558_000

@illwieckz
Copy link
Member Author

I added a commit adding an r_cheatLighting cvar, once enabled all light modes can be used (fullbright, vertex light, grid light, light map), but when disabled (by default), only vertex light and light map can be used, not selecting light map will fallback on vertex lighting.

@illwieckz
Copy link
Member Author

This looks ready to me.

@illwieckz
Copy link
Member Author

I know one map that glitches with this branch, it's nova. But nova was also glitching with Tremulous OpenGL 1 renderer, so actually glitching is being compatible and rendering things as expected. I don't know yet why nova glitches, but this may be a bug in the map data.

In 0.54.1 the glitch was not visible because of a bug disabling the features that glitches.

@illwieckz
Copy link
Member Author

We may actually name the cvars r_lightMode and r_cheatLightModes.

@illwieckz
Copy link
Member Author

We may actually name the cvars r_lightMode and r_cheatLightModes.

This sounds far better, I did that.

@illwieckz illwieckz changed the title Introduce r_lighting, implement vertex lighting and use it when needed. Introduce r_lightMode, implement vertex lighting and use it when needed. Jan 8, 2024
@slipher
Copy link
Member

slipher commented Jan 8, 2024

I don't think this 'cheat lighting' cvar helps anything. Better to do like

if ( !Com_AreCheatsAllowed() && r_lightmode.Get() == FULLBRIGHT )
{
    r_lightmode.SetValue(MAP);
    Cvar::Latch(lightmode);
}

@illwieckz
Copy link
Member Author

OK I used Com_AreCheatsAllowed, but I don't think changing the cvar value is a good idea.

I also turned the black list into a white list.

@slipher
Copy link
Member

slipher commented Jan 8, 2024

OK I used Com_AreCheatsAllowed, but I don't think changing the cvar value is a good idea.

That's how every other cheat cvar works. E.g. if you set cg_thirdperson 1 then connect to a server, it gets changed to 0.

@illwieckz
Copy link
Member Author

That's how every other cheat cvar works. E.g. if you set cg_thirdperson 1 then connect to a server, it gets changed to 0.

I would prefer to return to previous value but I guess it's not decidable.

@illwieckz
Copy link
Member Author

I did as you said.

@cu-kai
Copy link
Contributor

cu-kai commented Jan 10, 2024

@cu-kai in a recent game on “over the hill” map I read you saying “this is why r_lighting should not be cheat”, can you elaborate?

I think this was almost a joke in reference to dretches blending in with the texture used in the hall on that map.

@illwieckz
Copy link
Member Author

@cu-kai in a recent game on “over the hill” map I read you saying “this is why r_lighting should not be cheat”, can you elaborate?

I think this was almost a joke in reference to dretches blending in with the texture used in the hall on that map.

OK, thanks for the answer. 👍️

@illwieckz
Copy link
Member Author

10 23:20 <illwieckz> do someone has anything against https://github.com/DaemonEngine/Daemon/pull/992 ?
10 23:30 <cu-kai> i don't really have anything to say on this, i am not qualified to comment
10 23:36 <illwieckz> no one is qualified to comment :-/
10 23:37 <illwieckz> well, perturbed-mob has some interesting remarks about the cvar interface, and I believe their are now all addressed
10 23:39 <perturbed-mob> Lgtm 
10 23:39 <illwieckz> THX

@illwieckz
Copy link
Member Author

Today is a great day! 🎉️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Renderer T-Bug T-Feature-Request Proposed new feature T-Improvement Improvement for an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants