-
Notifications
You must be signed in to change notification settings - Fork 57
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
Goldsrc support completion #386
base: brushbsp
Are you sure you want to change the base?
Conversation
- add a new light option, "-surflight_rescale" to control whether lights have the haloing fix - change places expecting "trigger" to "aaatrigger" - treat "aaatrigger" surfaces as nodraw (matches VHLT) - set a bunch of defaults to better match VHLT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
everything else is looking good so far
light/surflight.cc
Outdated
@@ -203,7 +203,7 @@ static void MakeSurfaceLight(const mbsp_t *bsp, const settings::worldspawn_keys | |||
if (extended_flags.surflight_rescale) { | |||
setting.rescale = extended_flags.surflight_rescale.value(); | |||
} else { | |||
setting.rescale = is_sky ? false : true; | |||
setting.rescale = is_sky ? false : cfg.surflight_rescale.value(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be:
setting.rescale = cfg.surflight_rescale.is_changed() ? cfg.surflight_rescale.value() : !is_sky;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced it with that in an upcoming commit
Working on getting surface light emission looking more how I'd expect, I noticed that ZHLT and VHLT correct surface light colour values to linear from SRGB, which isn't performed by these tools right now. To make this easier to do, I've swapped most places that refer to surface light colours over to 0-1 qvec3f rather than 0-255 qvec3b (seems bounce lighting already uses 0-1 qvec3d). Converting from SRGB to linear is the correct thing to do when sampling the average colour from a texture, or reading light colours, but is quite tricky due to no gamma correction being performed at lightmap output time by default (gamma 1.0). Half-Life expects the final lightmap to be gamma corrected by a factor of 1.8 (or 2.0 in the original tools). Which means that when gamma correcting the lights which aren't already converted to linear, they get converted from SRGB > gamma 1.8, which is not correct at all. Basically, the problem I'm facing now is that colour isn't handled correctly by the tools, and if it is handled properly, then the default output will look wrong due to no correction if you don't specify a gamma > 1.0. Paril and I talked briefly about the gamma situation in the Quake mapping discord, right now the broken behaviour has become standard, basically Quake engines expect the lightmap in SRGB space, but historically the tools have always output in linear space (except in Half-Life). I have a couple of questions about the code that determines the intensity and colour of surface / bounce lights: Right now it looks like this, where // Calculate intensity
float intensity = qv::max(texture_color);
// Normalize color...
if (intensity > 1.0f)
texture_color *= 1.0f / intensity; This appears to match the // Calculate intensity...
float intensity =
texture_color[0] +
texture_color[1] +
texture_color[2];
// Normalize color...
qv::normalizeInPlace(texture_color); The second solution produces slightly different (but still correct) results when the pipeline is gamma correct, if the pipeline isn't correct, then it's totally broken. I don't know if this is technically the right thing to do, or if I'm just wrong. For some eye candy, here's a screencap of the output from VHLT, and the output from the tools after my changes, pretty much identical: |
How does the colornormalize change affect colored lighting? Can you maybe provide a small test case for it? |
color: - Introduced a new compile option for guaranteeing linear input and SRGB output. This is not fully hooked up yet. - Swapped several places in the code to use qvec3f over qvec3b, this makes it easier to handle SRGB versus linear color especially in places where the data is always evaluated as float color - Convert texture average colors to linear from SRGB, this makes sense when the final lightmap output is converted to SRGB (which should happen) but right now that is not the case by default, this MUST be changed in a later commit to be based on "srgbpipeline". - Convert surface light override colors to linear (.wal.json, lights.rad), as the input is provided in SRGB space, this matches the behaviour of ZHLT and VHLT, but must be controlled by "srgbpipeline" in a future commit. - Added experimental code that is #ifdef'd out to enable an alternative method of calculating texture intensity and normalization. goldsrc: - Rudimentary support for goldsrc light entities, light scale needs tweaking as they are too bright right now. All these additions combined produce output that is both visually correct and matches VHLT.
Sure, here's a link to a test map and a .rad file to go with it: testmap.zip Just pushed the code so far to the branch, which is needed to test this properly. The new method more closely matches other compile tools, which gives me a hunch it's probably more correct. |
Did a little bit more testing, the "new" method for calculating intensity and normalizing color is definitely not right. At the very least, converting everything to linear before doing anything produces colours in the final lightmap that more closely match the input colours, though I need to make this optional. |
Question about the "overbright" checkbox: In Q1/Q2, the original engine's rendering is with byte 127 = 100% brightness and 255 = 200%. There's a limitation in the original GLQuake where 128 through 255 were clipped to 100%. Engines like Quakespasm have a So in the Q1 context, "overbright off" means clipping 128..255 to all be 100% brightness, and "overbright on" would be 255 mapping to 200%. It looks like the "overbright off" option here instead does 128 = 50% and 255 = 100%? Is that a convention used in HL engines? |
Half-Life is a bit unusual with how it processes the lightmap, the final output is a little bit darker than typical quake engines, right now by default it also clamps the lightmap brightness in a really poor way, where colours become washed out and completely white after a certain threshold. The overbright checkbox can probably be reverted, I added it to more easily see the exact final output of the lightmap without the overbright multiplier added on top, though it didn't really help in the way I wanted. |
Regarding the overbrights discussion, ZHLT/VHLT's default overbright compensation limiter thingy kicks in at 188 brightness (modifiable with |
Also, might be better to treat any texture name starting with |
There appears to be multiple factors contributing to unintuitive behavior of the lightmap. Aside from the aforementioned gamma inconsistency, the lightstyle feature inherited from Quake does not seem to modulate to 100% by default (at least this is the case in Xash3D, which I assume is a proper imitation). It's not a significant deviation, so it would mostly manifest as some added unnecessary banding. I don't think there is any way to get around this, for my cvar manipulations have been in vain. I definitely think the VHLT overbright correction limiter's default state should be reverted. A lot of mappers will potentially return to Half-Life with the recent update where shaders are enabled by default. It would only harm non-HL maps a bit, but it would harm new HL maps a lot more not to. |
Random shower thought: since the engine discards all water faces (those textured with
Only in the base game, and server owners can disallow them via a cvar (bandaid fix for avoiding wallhack shaders). Lighting looks a lot worse when unclamped without overbright than when clamped when overbright. |
Hey @Slartibarty 👋🏻 What's missing to get this merged? Is there anything specific we can do to push this forward? |
Still missing support for HL light entities, like light_spot, and there needs to be a heuristic to convert HL light values to what the tools currently expect (intensity of 300 is different in ZHLT and these tools). I've been occupied with other stuff since starting this PR, and I dug myself into a hole by trying to refactor the tools use of colour at the same time, which was a mistake and should be saved for another PR. Initially I started working on this since there was a passing mention that peeps at Valve might be interested in using the tools for the HL 25th anniversary update, but they ended up using the VHLT toolset, which hit my motivation to work on this PR. |
Hey. No pressure at all! Thank you for your commitment on this. I am so sorry you felt unmotivated. I still believe your work is valuable and will benefit. I am personally looking to create a Goldsrc Level Design Starter Kit; including ericw-tools instead of ZHLT (or VHLT) will probably be a game changer for the community (because of all the advanced light features the tool offers). |
Support for goldsrc is 90% of the way there, but some features are missing to allow for easier migration from ZHLT or VHLT.
I'm creating this as a draft PR to allow for discussion and whatnot.