renderer: implement complete overBright in GLSL and high-precision frame-buffer

[illwieckz]

• renderer: implement complete overBright in GLSL

We cannot overBright separate stage as stage are clamped within [0.0, 1.0] when blending multiple stages together, meaning the result of the multiplication of a separate light stage with a light factor will be clamped before blending it with the color map.

This implementation implements overBright in the camera shader, but to only apply this overBright to surfaces having received lights, surfaces that do not receive lights gets divided by the light factor in a way re-multiplying them cancels the division.

Stages and surfaces to be blended over other stages or surfaces are also divided to avoid multiplying multiple time the same surface, in order to have the final result being (m * a) + b instead of m * (a + b) wich would be (m * a) + (m * b) and then sump up the factor, something we don't want to do.

The legacy code for overbrighting (but clamping) the light at BSP load is kept for when the feature is disabled, but rewritten to make the code better.

This feature can be enabled by disabling the r_mapClampOverBright cvar.

• renderer: implement high precision rendering framebuffers

This is needed to avoid color-banding with the various division/multiplication round trips of the complete overBright implementation. This will also be needed for sRGB colors in the future too. It is enabled by default if a feature requires it.

This is optional and can be disabled with r_highPrecisionRendering, or automatically disabled if the hardware doesn't support the feature.

[illwieckz]

Complete overBright in GLSL is currently disabled by default, the code is considered good enough to be enabled by users to evaluate it and report issues. This may be improved incrementally in the future but I haven't spot a single error myself yet.

Maps like terminus and area3 are not rendered properly (too bright) but that's not because of a bug in this code. This is because we render them with mapOverBrightBits feature being enabled but those maps were initially made for a game disabling mapOverBrightBits by default. This can be fixed by setting the "mapOverBrightBits" "0" key in the worldspawn entity of these maps, or if the clamped overBrightBits pleases the eye, keep the overBright but clamp it by setting the "mapClampOverBright" "0" key in the worldspawn entity.

[illwieckz]

Future sRGB code will require this complete overBright implementation and will also need high precision frame buffer for better results. All this code will be enabled by default when loading a map with sRGB, even if the r_mapClampOverBright cvar is enabled.

[illwieckz]

This was tested with maps like:

• test-portal-recursion (portals and mirrors).
• atcshd (legacy materials, light known to be broken by clamped overBright)
• atcszalpha (legacy materials, light known to be broken by clamped overBright)
• metro (legacy materials, light styles)
• pulse (legacy materials, light styles)
• plat23 (state of the art materials)
• Wolf:ET's oasis (legacy materials, global fog, mixed light map and vertex light)

No visual regressions and glitches were noticed.

[illwieckz]

Same question as #1035 (review) - among lightmaps/light grids/vertex lighting which ones is the new algorithm expected to apply to?

All of them. There should be overbright anytime there is a precomputed light.

With that reverse implementation there is overbright cancelling anytime there is no precomputed light, in a way multiplying the final result reverts non-lit surface to their original colors, and multiplies lit surfaces to their overbright colors.

That just makes me think we may have to divide the dynamic lights, as those are expected to not be multiplied. The overbright light factor is for precomputed lights only.

[illwieckz]

That just makes me think we may have to divide the dynamic lights, as those are expected to not be multiplied. The overbright light factor is for precomputed lights only.

Dynamic lights are now divided to cancel overBright on them.

[illwieckz]

So, what people can do (just to exclude bugs from other buggy features):

• check that r_dynamicLight is 2
• check that cg_shadows is either 0 or 1, not more

Then:

• set r_mapClampOverBright to off

And play public games this way, and report which map break. This way we can decide to disable it by default or not.

I expect maps from Tremulous being all fine, but some maps built for Unvanquished having bugs. The thing is that mappers have tested against their maps with a broken lighting and then, mappers were not aware they introduced some bugs.

Example of maps I spotted:

map	bug	proposed fix
fortification	outdoor surfaces are too bright	set mapClampOverBright 1 in BSP worldspawn

The two maps ported from Urban Terror are also buggy, but that's expected: we set mapOverBrightBits 2 on them by default but the original game expected 0. It happens that mapOverBrightBits 2 is not bad with the terminus map when overbright is clamped, so we may do that instead.

map	bug	proposed fix
area3	outdoor surfaces are too bright	set mapOverBrightBits 0 in BSP worldspawn
terminus	many surfaces are too bright	set mapClampOverBright 1 in BSP worldspawn

[illwieckz]

Actually freeway having “too bright windows“ may be due to something wrong: the glow maps are multiplied but we may not want to do this. Problem is, with collapsed stages, we know what is a glow map, but with legacy stages, especially if the engine fails to collapse them, we don't know what is a glow map.

I thought today of an alternate implementation for GLSL not-clamped overBrightBits: the reason why I multiply everything at the end but divide what should not be multiplied to cancel the multiplication is that framebuffers are clamped to 1, so if we multiply a legacy light stage, it will be clamped to 1 before blending the texture over it, so it's as bad as if we clamp the lightmap at load time.

But, what I thought today is that since the engine max overBrightBits is 3, we may divide all stages by 2^3, then multiply the final result by 2^3, then we would only multiply the lights, only the lights. Because the stage would always be divided before blending, and divided with a value greater or equal to the overBright multiplier, it will never be clamped. The good thing is that we always know what is a light.

[illwieckz]

I cancelled overBright on collapsed glow maps.

It looks like I already cancelled on separate stages one, in fact they are simply detected as other blended surfaces that should not be overBright, without needing to know they are glow stages.

The remaining problem with freeway map seems to be that bloom is multiplied. There is no issue in freeway map anymore.

[illwieckz]

Bloom overbright is now properly cancelled.

[illwieckz]

Here is a combination of this branch and the ATCSHD remaster @RedSkyByte is working on:

[DolceTriade]

Been running this for a bit, and I haven't noticed any regression. The only oddness I have seen is in the map Archao, but that oddness was present on master too (flickering textures at the spectator spawn point).

[illwieckz]

What do you think about renaming mapClampOverBright to legacyMapClampOverBright? The clamping is only a backward compatibility purposed for being compatible with bugs of legacy maps.

If you don't mind, I think I will merge this soon if no one object. Not having this is blocking the work on sRGB/linear colorspace:

• https://github.com/DaemonEngine/Daemon/pull/1034

The clamping will be automatically disabled on maps using the sRGB/linear colorspace in all cases, meaning all newly built maps will use that code.

After this is merged, there can be other improvements done:

• Disable the legacy map overbright clamping by default, I tested many maps, and it's fine. In case we stumble upon a map requiring those bugs, we can repackage it with the key to enable the clamping.
• I may evaluate some redesign of the feature in the future with some fresh ideas. We don't need to wait for such revamp to merge this, this implementation works.

[slipher]

I insist that we should use "default" in the cvar names lest users be surprised when the cvar fails to override the value set in the worldspawn. r_defaultMapOverbrightBits and r_default(Map)ClampOverbright. (I don't get why the word "map" should be included in the worldspawn key name since there is not any other kind of "clamp overbright" that it needs to be distinguished from.)

[illwieckz]

r_defaultMapOverbrightBits

We better not want to rename r_mapOverBrightBits. This one was kind of public API for games to configure the engine.

[slipher]

Have you tested with different values of r_lightMode? I tried fullbright mode with clamping off and it looks really bad.

[illwieckz]

Have you tested with different values of r_lightMode? I tried fullbright mode with clamping off and it looks really bad.

Ah right, the fix for it was lost in my multiple rewrites. This is now fixed.

[illwieckz]

For r_mapClampOverBright, I think we may just name it r_forceLegacyMapClampOverBright and disable the clamping by default. I can also name the map variable legacyMapClampOverBright, as it is not expected to be used by non-legacy maps and will be ignored if used in non-legacy map.

The map variable would tell the renderer to use the legacy code path as a workaround for maps that has may look better with the old buggy code because they were tested with the old buggy code, and the cvar would be usable by the user to test a map with the old buggy code to test the workaround before setting the variable in the map.

Actually, I now don't see good reasons to not disable clamping by default in all maps. Once people start to play with not-clamped overbright, any legacy map with clamped overbright looks broken, much more than the quirks that may be revealed by not clamping the lights.

We were not seeing the bugs because we never seen a map properly rendered before, but once we see a map being properly renderer, the eyes then see the bugs on almost every surface when clamping is enabled. We can't unseen those light bugs once seen, and they are everywhere.

Configuring a map to force the legacy code would require such map to be strongly broken, and for now the only maps that are that broken are maps from another game that used a game using different renderer default values.

Also a r_forceLegacyMapClampOverBright cvar would be useful when debugging regressions from Tremulous. For example it looks like our fog is wrong, so if someone wants to debug the fog and have a fair comparison with Tremulous, that one may want to work with a similarly buggy renderer to not be fooled by our fix. For example if a fog color was clamped, we should test the new implementation first with clamping on to test for visual parity, because testing for visual parity with clamping off on our end may lead us to re-implement clamping in fog code itself.

[illwieckz]

So, the cvar to enable old clamped code base is r_forceLegacyMapOverBrightClamping, it overrides any map option. It is only meant to be a diagnostic tool to have a fair comparison when comparing renders with engines having old clamped code (be backward compatible on bugs when comparing engines).

The map related option is forceLegacyMapOverBrightClamping and can be used as a per-map option to tell the renderer to use that old code path as a workaround. I'm not sure this variable will be kept forever, we may remove it in the future if it turns out it is never needed.

The complete overbright is now enabled by default for all maps. One can't unseen the buggy clamping after having played on maps without buggy clamping.

The r_mapOverBrightBits cvar name is kept this way for backward compatibility with legacy game configs when loading third-party game assets when evaluating the engine for running third-party games or loading third-party game data.

I now consider this PR totally ready (after I squash some fixup commits).

[sweet235]

This branch is addictive. Since I discovered it a few days ago, I have spent hours just looking at maps.