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

Add support for BSP "LIGHT" surfaces #111

Merged
merged 19 commits into from
Aug 9, 2021
Merged

Conversation

res2k
Copy link
Contributor

@res2k res2k commented Mar 25, 2021

For lighting, Q2RTX predominantly uses light-emitting surfaces. The amount of light a surface should emit is determined by an entry in the materials list and an "emissive" texture associated with a material. Material definitions and, if appropriate, emissive textures are provided for basically all baseq2 materials.
Unfortunately, if an emissive texture is not present, a surface won't emit light. This is a problem for maps that use materials beyond the baseq2 set - ie from mods or mission packs (see also #77).
Fortunately, the original light information is still present: the SURF_LIGHT flag indicates that a surface should emit light.
With this information "old" maps could be lit much more like they were intended to.

This patch adds a check for LIGHT surfaces and the ability to synthesize an appropriate light-emitting material.
An emissive texture is guessed from the diffuse texture by taking the "brightest" pixels. This looks okay for textures in the typical Quake 2 "wall light" style. (An earlier approach was to use the diffuse texture only. While useable, it's not the most attractive look.)

Additionally, while trying out the changes, I noticed:

  • Quite rarely there's a surface with a LIGHT flag, and it's texture has a materials.csv entry, but it lacks the emissive texture (happens in eg base3). This probably indicates an "artwork problem" - missing emissive texture - that results in surfaces not emitting light, though they were originally intended to.
  • Water and slime are supposed to faintly emit light! This makes a visible difference in some cases (eg base1 cellar). To see the difference set pt_enable_surface_lights_warp to 1. I left it off by default since mainly because it's a hack, having water emit light without light textures. That "proper" way is also available (set both pt_enable_surface_lights and pt_enable_surface_lights_warp to 2), but the result looks quite weird/jarring/unappealing.
    FWIW, the faint "water light" might very well be relevant in certain dark areas, so it might be good to find a way to have it back (without hacks).

@res2k
Copy link
Contributor Author

res2k commented Jun 7, 2021

Hey @apanteleev, what's your opinion on this?
For one, I'm unsure whether PRs like this, to "make old stuff work", is something you consider worthwhile for Q2RTX, or if you want to focus on "baseq2"-related stuff only...

@aepod
Copy link

aepod commented Jun 7, 2021

This is an amazing PR, and will fix many Maps throughout many mods. For instance Lokis Minions almost all maps use emissive textures as lights, so in RTX they are far too dark. Without going through and manually updating maps this is impossible to fix. This fix solves that. There are literally thousands of maps that will possibly fixed by this PR.

Kudos!

@apanteleev
Copy link
Collaborator

Hey @apanteleev, what's your opinion on this?

Well, in general, I wouldn't mind merging this. But one thing I dislike about the implementation is that there seems to be a significant amount of image processing being done on the CPU at map load time. Maybe it's insignificant, but I feel like our loading process is slow enough already, with all the image extraction from .pkz and decoding being done in a single thread. Have you measured the impact of this processing on load times?
(Actually, it probably doesn't matter for baseq2 because the synthesis is disabled by default through the new cvars, right?)

@res2k
Copy link
Contributor Author

res2k commented Jun 8, 2021

Well, while the main cvar is "on" by default, it shouldn't matter too much for baseq2, as the mechanism doesn't do any image processing if there's a material.csv entry for the texture, which (to my knowledge) all baseq2 wall textures have.

Yes, image processing on the GPU is quite sensible. But the CPU approach more easily fits into the current "upload a bunch of pixels" model, while a GPU processing component will need some enhancements to the texture uploading mechanism.
But in general, it's certainly an enhancement I'm interested in doing.

Thanks for your feedback. It's good to know directions to get my ideas shared with the world :)

(Unfortunately I don't have hard numbers on the load time difference. I can't quickly produce some right now either - there's no "map load time" measurement built in, and subjectively, the difference is too small to measure manually. Perhaps reporting a "map load time" is a worthwhile addition in itself.)

@apanteleev
Copy link
Collaborator

apanteleev commented Jul 24, 2021

I've tested this PR with the current master, and it works... in some cases. Not very reliably. Like, in Ground Zero it mostly doesn't work, I think because the 215 unit threshold for brightness is too high for many of the textures. Maybe introduce a cvar? Also, ignoring the baseq2 materials is not always a good solution.
I'll take another look at the problem later.

Edit: the change about ignoring baseq2 materials was in another PR that I already merged. Oh well.

@res2k
Copy link
Contributor Author

res2k commented Jul 24, 2021

My go-to test was mostly some maps of xatrix (The Reckoning), and it seemed fined there... but yeah, it's very texture-dependent (and also subjective).

Yes, a cvar would a good way to quickly see what effects different thresholds have.

What would probably be best would be to have a database that allows tweaking of the threshold (maybe other parameters) per texture.
But with the sheer amount of mods, maps out there... well, I suppose a robust automatic solution would be much more manageable ;)

In an earlier iteration I had this:
float threshold = max_lum * 0.68f;
...perhaps an approach of "guessing" the threshold could produce appealing results for a wider range of textures.

BTW, did you mean this: res2k@2415da2 with "ignoring baseq2 materials"?
Generally, if the change in question is from a PR of mine, I can probably explain the reasoning behind it ;)

res2k added 16 commits July 24, 2021 13:56
Required for proper reloads with custom materials present.
We're using "just" MAT_RegisterPBRMaterial() for synthesized materials,
even when reusing a previous material - but that case needs an image
registration sequence update.
So just "update" a material upon registration.
Previously, the algorithn was to simply extract pixels above a certain dynamic brightness.
Some scaling and blurring was applied as well.
The results were usable but could be crude-looking.

The new approach determines all 'bright' pixels (largest component above a certain) and
applies some blurring as well as some filtering with the original image.
After some experimentation with the details this resulted in an algorithm that
seems to extract emissive images that seem to be relatively faithful to the appearance
of the "light" part of original textures.
@res2k
Copy link
Contributor Author

res2k commented Jul 25, 2021

@apanteleev Also check this commit: 937fb77, may affect your experience with Ground Zero.

Also, if which levels in Ground Zero did you look at? I'd like to check them out as well.

@apanteleev
Copy link
Collaborator

which levels in Ground Zero did you look at?

Just the first one, rmine1. It was pretty dark.

@res2k
Copy link
Contributor Author

res2k commented Jul 25, 2021

Just the first one, rmine1. It was pretty dark.

Please try again with the latest change, this improves lighting there quite dramatically.

When looking for an override, don't prefer 'game' images over 'base' images.
This is more or less another workaround for games that duplicate base textures;
in that case, we want to preferably use any high-res overrides and
it's additional images.
@apanteleev apanteleev merged commit e8d1a5c into NVIDIA:master Aug 9, 2021
@res2k res2k deleted the LIGHT-surfaces branch September 20, 2021 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants