renderer: merge lightmap and vertex (lightgrid) world and entity lighting #301

illwieckz · 2020-03-09T23:24:20Z

This merges world lightmapping glsl, world lightgrid lighting glsl (wrongly called vertex world) and entity lightgrid glsl (wrongly called vertex entity).

This prevent the risk of mistakes by not copy pasting 90% of the same code between files. This also helps to improve feature parity between all rendering options.

It makes possible to do or to implement:

normal mapping on maps without deluxe maps (useful to improve legacy maps)
deluxe mapping disablement to rely on lightgrid direction instead (useful for debugging purposes, this may also use less gpu memory)
render some world part with lightmap and other world part with lightgrid like moving platforms to avoid moving shadows and to enable ambient lighting on them instead.
lightgrid lighting with deluxe map-based normal lighting (I don't see any usage, but it would work)
do reflective specular on world (what is it?)

Also it would be possible to implement rim lighting on world but I don't think there is any usage for that, in any way it would be very easy to do.

The implementation is rendering properly world and entities using either light maps or light grid, and normal maps are rendererd properly using either deluxe maps or light grid.

illwieckz · 2020-03-09T23:28:10Z

Once this is merged, merging cube reflection within diffuse lighting stage would only require to edit one glsl code instead of three, same for any future feature we would want to add.

Maybe one day we would be able to also merge the liquid code.

illwieckz · 2020-03-09T23:55:57Z

The “invisible surface” bug related to viewing angle is probably not in GLSL code.

The bug is probably not in lightMapping_fp.glsl file (fragment program) since replacing the whole main by a simple 3-èline code displaying the diffuse texture is reproducing the bug. There is very low chance the bug would be in lightMapping_vp.glsl file (vertex program) since this file is small and simple.

On C++ side, there is low chance the bug would be in gl_shader.cpp or gl_shader.h (code is not very complex). So there is high chance this code is in tr_shade.cpp.

illwieckz · 2020-03-10T00:59:21Z

One interesting thing is that the code stores lighgrid1 (used to compute light color) as lightmap when lightmap is disabled or unavailable, and stores lightgrid2 (used to compute light direction) as deluxemap when deluxe map is disabled or unavailable. This way there is only two binds for light color and light direction whatever the combination of features used. Then some #ifdef enable the related code in GLSL.

- #317 with 8225d6 required r_deluxeMapping to be latched - #301 will require it too to tell tr_bsp to load deluxemap even if lightmap is disabled

illwieckz · 2020-04-16T15:51:30Z

PR is now ready, #309 was required to make it working.

There is now a single function in tr_shade.cpp and glsl shader for lightmapped world, grid-lit world, and grid-lit entities. They share now the same feature, and they work the same or fail the same.

Those scenarii are now possible:

lightmap-based light mapping and deluxemap based normal mapping,
the usual scenario with official map;
lightmap-based light mapping and lightgrid based normal mapping,
would allow use to add normal map to already baked legacy maps without deluxe maps;
lightgrid-based light mapping and deluxemap based normal mapping,
this is a curiosity made possible for free;
lightgrid-based light mapping and lightgrid based normal mapping,
for when lightmapping is disabled, also used by entities.

At this point, it is expected to reproduce existing bugs. Scenario 2. currently looks wrong. The fix seems to be easy: apply on lightgrid color used for normal mapping the same tweak we apply on lightmap and discussed in #299 and #306 fixes it, but then entity lighting may be wrong, at least not what we are accustomed to. It looks like light grid lighting is wrong and too much bright (see also #313).

Note: the fact the same code is used for entities and map models would allow us to avoid the bug of moving models with moving shadows by lighting moving models with lightgrid, it would also mean moving map models would have their lighting changing with ambient light, which would be very cool.

illwieckz · 2020-04-16T15:55:32Z

This PR deletes almost 680 lines, does not remove features, allow more feature combinations, make new features possible, make the code more tested, will make maintenance easier, and would prevent some bugs to be introduced or fix forgotten by oversight.

illwieckz · 2020-04-16T17:31:49Z

I added a commit to fix the lighting on the “lightmap light color but lightgrid light direction” scenario. I still thing there is an issue with lightgrid light color but that would be something else.

illwieckz · 2020-04-16T17:33:46Z

So, the current status:

lightmap-based light mapping and deluxemap based normal mapping,
the usual scenario with official map: OK, like before;
lightmap-based light mapping and lightgrid based normal mapping,
would allow use to add normal map to already baked legacy maps without deluxe maps: OK, new;
lightgrid-based light mapping and deluxemap based normal mapping,
this is a curiosity made possible for free: NOT OK, useless;
lightgrid-based light mapping and lightgrid based normal mapping,
for when lightmapping is disabled, also used by entities; NOT OK, as usual.

Fix for 4. is out of topic of this PR. At least this PR does not change legacy behaviour.
3. would be probably fixed by fixing 4. but no one cares anyway.

illwieckz · 2020-04-16T18:14:09Z

Lightmap-based light color with deluxemap-based light direction:

Lightmap-based light color with lightgrid-based light direction:

Lightgrid-based light color with deluxemap-based light direction:

Lightgrid-based light color with lightgrid-based light direction:

The one before the last one is obviously wrong but we don't have to take care about it at this time.

This last one does not looks that bad, but it's a bit too much bright, that can be seen especially on the building. See #313.

src/engine/renderer/tr_local.h

slipher · 2020-04-18T22:23:43Z

BIND_ sounds good. Feel free to go ahead. As you can see, the PR is way too complicated for me to understand, so all I could do is bikeshed the enum naming.

Kangz

Looks like a pretty good cleanup!

src.cmake

src/engine/renderer/tr_local.h

…ting

illwieckz · 2020-04-23T15:56:44Z

I currently disabled the feature to do normal/specular mapping on bsp surfaces because of #324.

It would be cool to fix #324 to revert this change, even if we don't care about that feature: this workaround is very intrusive and I would like to get rid of it.

This workaround makes sure those surfaces are rendered the same before and after the merge is done.

See also #320 for a related issue about lighting that may be related to #324, and #299 for the deluxemap counterpart of #324.

illwieckz · 2020-04-23T20:53:06Z

I also noticed current master fixed #326 (blend issue with internal lightmap), probably thanks to #309 (tell a surface belongs to the bsp, properly select the renderer). But the light map/light grid code merge re-introduced a variant of it.

So that was likely an issue related to the fact that when the code for grid lighting and light map lighting is merged, we have to take care of the fact the light grid must not be used if there is no lightmap but light mapping is disabled. Because of the design, no lightmap use lightmap code but with a white image.

So I added a commit to:

enable lightmapping with whiteimage on nolightmap shader, fix Blend issue with internal lightmap #326;
enable lightmapping with whiteimage when there is no lightmap neither lightgrid;
enable deluxemapping with blackimage when there is no deluxemap;
rework the code and functions around those features.

…en with lightgrid direction, ref DaemonEngine#299, DaemonEngine#306 The feature discussed in DaemonEngine#299 and DaemonEngine#306 to restore original light color to feed the normal-based light computation was only applied on surface using deluxe maps, leading to anomalous dark lighting with maps without deluxemaps, or when deluxe mapping is disabled. It's not related to deluxe maps but to light maps any way

… color but missing deluxe map direction, ref DaemonEngine#324 Workaround DaemonEngine#324, restore the unavailability of this feature.

…shader, fix DaemonEngine#326 - enable lightmapping with whiteimage on nolightmap shader, fix DaemonEngine#326 - enable lightmapping with whiteimage when there is no lightmap neither lightgrid - enable deluxemapping with blackimage when there is no deluxemap - rework the code and functions around those features

…ightMapping Render_lightMapping is the only function using GetLightMap and GetDeluxeMap

illwieckz · 2020-04-23T21:47:56Z

Let's do it.

illwieckz added S-Help-Wanted A-Renderer T-Cleanup labels Mar 9, 2020

illwieckz force-pushed the glslmerge branch from fe0119e to 3c2aed7 Compare March 9, 2020 23:50

illwieckz force-pushed the glslmerge branch from 3c2aed7 to 0e0a6fa Compare March 10, 2020 02:35

illwieckz mentioned this pull request Mar 12, 2020

need a reliable way to detect if a given surface is one of a bsp model #302

Closed

This was referenced Apr 11, 2020

tell a surface belongs to the bsp, properly select the renderer #309

Merged

renderer: some minor improvements (factorisation etc.) #300

Merged

illwieckz added a commit that referenced this pull request Apr 16, 2020

renderer: set r_deluxeMapping latched value, ref #317, #301

f500210

- #317 with 8225d6 required r_deluxeMapping to be latched - #301 will require it too to tell tr_bsp to load deluxemap even if lightmap is disabled

illwieckz force-pushed the glslmerge branch from 0e0a6fa to 3d3fbc3 Compare April 16, 2020 12:14

illwieckz changed the title ~~WIP: renderer: merge lightmap and vertex (lightgrid) world and entity lighting~~ renderer: merge lightmap and vertex (lightgrid) world and entity lighting Apr 16, 2020

illwieckz mentioned this pull request Apr 16, 2020

renderer: factorize and move some computation done every frame for every surface in tr_shade to be done only once in tr_shader #317

Merged

illwieckz removed the S-Help-Wanted label Apr 17, 2020

slipher reviewed Apr 18, 2020

View reviewed changes

src/engine/renderer/tr_local.h Show resolved Hide resolved

Kangz approved these changes Apr 19, 2020

View reviewed changes

src.cmake Outdated Show resolved Hide resolved

src/engine/renderer/tr_local.h Show resolved Hide resolved

illwieckz force-pushed the glslmerge branch 2 times, most recently from 0a1c05a to a2d234e Compare April 21, 2020 14:24

illwieckz added 2 commits April 21, 2020 16:25

renderer: merge lightmap and vertex (lightgrid) world and entity ligh…

2254a4f

…ting

renderer/tr_bsp: always load lightmap if deluxemap if enabled

42ebd4b

illwieckz mentioned this pull request Apr 21, 2020

Fixing the lights, something does not look legit in the code #320

Closed

illwieckz force-pushed the glslmerge branch from a2d234e to 6d69df5 Compare April 21, 2020 15:24

illwieckz mentioned this pull request Apr 21, 2020

Broken lantern/candle in metro map, light emitting shader is displayed #322

Open

illwieckz force-pushed the glslmerge branch from 6d69df5 to 9130b08 Compare April 23, 2020 15:53

illwieckz mentioned this pull request Apr 23, 2020

Blend issue with internal lightmap #326

Closed

illwieckz added 4 commits April 23, 2020 23:01

renderer/lightMapping: disable normal/specular compute whith lightmap…

572d11a

… color but missing deluxe map direction, ref DaemonEngine#324 Workaround DaemonEngine#324, restore the unavailability of this feature.

renderer/tr_shade: move GetLightMap and GetDeluxeMap next to Render_l…

4d8a783

…ightMapping Render_lightMapping is the only function using GetLightMap and GetDeluxeMap

illwieckz force-pushed the glslmerge branch from 1851568 to 4d8a783 Compare April 23, 2020 21:07

illwieckz linked an issue Apr 23, 2020 that may be closed by this pull request

Blend issue with internal lightmap #326

Closed

illwieckz merged commit fc3a49b into DaemonEngine:master Apr 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

renderer: merge lightmap and vertex (lightgrid) world and entity lighting #301

renderer: merge lightmap and vertex (lightgrid) world and entity lighting #301

illwieckz commented Mar 9, 2020 •

edited

Loading

illwieckz commented Mar 9, 2020

illwieckz commented Mar 9, 2020 •

edited

Loading

illwieckz commented Mar 10, 2020

illwieckz commented Apr 16, 2020 •

edited

Loading

illwieckz commented Apr 16, 2020

illwieckz commented Apr 16, 2020 •

edited

Loading

illwieckz commented Apr 16, 2020 •

edited

Loading

illwieckz commented Apr 16, 2020 •

edited

Loading

slipher commented Apr 18, 2020

Kangz left a comment

illwieckz commented Apr 23, 2020 •

edited

Loading

illwieckz commented Apr 23, 2020

illwieckz commented Apr 23, 2020

renderer: merge lightmap and vertex (lightgrid) world and entity lighting #301

renderer: merge lightmap and vertex (lightgrid) world and entity lighting #301

Conversation

illwieckz commented Mar 9, 2020 • edited Loading

illwieckz commented Mar 9, 2020

illwieckz commented Mar 9, 2020 • edited Loading

illwieckz commented Mar 10, 2020

illwieckz commented Apr 16, 2020 • edited Loading

illwieckz commented Apr 16, 2020

illwieckz commented Apr 16, 2020 • edited Loading

illwieckz commented Apr 16, 2020 • edited Loading

illwieckz commented Apr 16, 2020 • edited Loading

slipher commented Apr 18, 2020

Kangz left a comment

Choose a reason for hiding this comment

illwieckz commented Apr 23, 2020 • edited Loading

illwieckz commented Apr 23, 2020

illwieckz commented Apr 23, 2020

illwieckz commented Mar 9, 2020 •

edited

Loading

illwieckz commented Mar 9, 2020 •

edited

Loading

illwieckz commented Apr 16, 2020 •

edited

Loading

illwieckz commented Apr 16, 2020 •

edited

Loading

illwieckz commented Apr 16, 2020 •

edited

Loading

illwieckz commented Apr 16, 2020 •

edited

Loading

illwieckz commented Apr 23, 2020 •

edited

Loading