This does seem like a hack around an engine bug... There shouldn't be any ambiguity; in an engine supporting lit water, a face with face->styles = 255, face->lightofs = -1, TEX_SPECIAL unset, and texture name starting with '*' should be rendered black, right?

It's not an engine bug, the lightmap data is missing from the BSP file itself. For some reason, if no lights touches a liquid surface, no lightmap data is generated for it. I've studied the light.exe code for hours and couldn't figure out what causes this, so I've just forced the lightmap data of liquid surfaces to always be generated.

With no lightmap data, the surface cache for the lighting can't be created.

I think this bit is missing from your patch?
It's not missing, I've just implemented it in a subtle way:

/* don't bother with light too far away */
if (dist > entity->fadedist)
if (!backwater) // mankrip
return;

This allows far away lights to be casted on liquid surfaces, therefore forcing the lightmap data to be generated - even though far away lights won't add any brightness to it. 

It's not missing, I've just implemented it in a subtle way
Ah, I see :)

It's not an engine bug, the lightmap data is missing from the BSP file itself. For some reason, if no lights touches a liquid surface, no lightmap data is generated for it
This is not specific to lit water.. light.exe will not write fully black lightmaps to the bsp. It was a compression / optimization in the original light tool and engine; lightofs of -1 is rendered black. You can see this by deleting all lights except a small dim one in a large map, the lightdatasize will be only be a few hundred bytes.

here's the early exit in light: (f->lightofs = -1; is set earlier)
https://github.com/id-Software/Quake-Tools/blob/master/qutils/LIGHT/LTFACE.C#L542

and places where it is handled in the engine:
sets surf->samples to NULL if face->lightofs is -1:
https://github.com/id-Software/Quake/blob/bf4ac424ce754894ac8f1dae6a3981954bc9852d/WinQuake/gl_model.c#L795 

This is not specific to lit water.. light.exe will not write fully black lightmaps to the bsp.

I know this.

I don't remember all the details on why the lighting for liquids couldn't be generated if the light data wasn't present in the BSP file, but I will take a look at it again later. 

Alright, I've had some sleep.

Here's the proof why the lightdata for liquid surfaces must always be generated. In Mod_LoadFaces:


if (!Q_strncmp(out->texinfo->texture->name,"*",1)) // turbulent
{
out->flags |= (SURF_DRAWTURB | SURF_DRAWTILED);


The SURF_DRAWTILED flag in the engine is similar to the TEX_SPECIAL flag in the tools: it serves to indicate that the surface shouldn't have lightmaps. And it can't be eliminated, otherwise maps compiled without -splitspecial will crash the engine when trying to generate the surface caches.

Right now, I'm using an if(!out->samples) statement to ensure that maps without lit water will have the SURF_DRAWTILED flag set. However, the engine loads the mtexinfo_t data through Mod_LoadTexinfo right before Mod_LoadFaces is called, so in theory it should be possible to replace the (!out->samples) check with a (texinfo.flags & TEX_SPECIAL) check.

I'll modify the light tool to remove the "always save lightdata" hack, compile a map with it and change the check in the engine to see if it works. 

Yeah, it worked. Now instead of this:


if (!Q_strncmp(out->texinfo->texture->name,"*",1)) // turbulent
{
out->flags |= (SURF_DRAWTURB | SURF_DRAWTILED);


... the proper way to set SURF_DRAWTILED is this:


// mankrip - begin
if (out->texinfo->flags & TEX_SPECIAL)
out->flags |= SURF_DRAWTILED;
// mankrip - end
if (!Q_strncmp(out->texinfo->texture->name,"*",1)) // turbulent
{
out->flags |= SURF_DRAWTURB; // mankrip - edited


:) Thanks for making me look into this. 

been fighting with this for a while now.. it's more or less in a finished state I think.

windows builds source

Command line flags:
-bounce enables 1 bounce (this is all that's supported)
-bouncedebug saves only the indirect lighting to the bsp/lit, for previewing
-bouncescale scales the brightness of bounce lighting, default 1. can try 2 or 0.5 for more/less indirect light.
-bouncecolorscale how much bounce lighting picks up color from the map textures, between 0 and 1. default 0 because this can lead to q2 style everything-is-gaudy. you can try 0.5 or 1.

Note, this is slow! light uses visdata now, I recommend only using -bounce on vised maps because that speeds it up a lot.

Other info: it works more or less like the existing "_surface" light system; point lights are generated on each face to bounce light back into the map. Currently there is no face subdivision for this, so the bounce light given off by really large faces will be coarse (all coming from just 1 point light). 

I'd like to see a screenshot showing the effect though. 

https://pbs.twimg.com/media/Cgwje1aWYAAn_Wf.jpg:orig

left is with bounce, right is w/o. 

mind you. 

without
with

"Without" shot used "-lit -extra", 9seconds
"With" shot used "-lit -extra -bounce", 39 seconds

the 39 seconds is with 8 threads, and vised map though. Just -bounce without -extra is something like 16 seconds. 

oh man I am SO hyped for this! 

 

I do kind of wish for a more tolerant set of compilers like rebbs. 

Ok, this seems to have fixed a couple of issues in my map so I will be using this :) 

cool.. glad it works.
FWIW mfx and I did a ton of back and forth testing/fixing on phong shading, it's pretty solid now (though one glitch just came up that I need to look into)

check the BJP tools thread, I linked a build of txqbsp that is patch to write the phong shading info. 

Thanks Eric, I am more inclined to use those simply due to my god awful brushwork. 

Looking at the current source, you've implemented support for properly lit water.

Thanks EricW, I'll try it out. 

It works flawlessly, way better than my implementation.

I've noticed -fence was removed, but that's not a big problem for me.

I'll try out making some smooth shaded lit liquids now. 

glad the lit water works :-)

-fence I removed because it was done in a hacky way. I understand the trace code better now so can re-add it if anyone wants it. 

 

Those screenshots are gorgeous! 

http://ericwa.github.io/tyrutils-ericw/

New features:

- light: added a better options summary with the -help flag
- light: added -bounce option, "_phong", "_project_texture" key
- light: use vis data to accelerate lighting
- light: "_minlight_exclude" key to exclude a texture from receiving minlight
- light: add "_sun2" "_sun2_color" "_sun2_mangle" which creates a second sun
(unrelated to "_sunlight2" which is the sky dome light)
- vis: support .prt files written by bjptools-xt
- qbsp: add -objexport flag

Bugfixes:

- vis: fix ambient sounds when using func_detail, broken in tyrutils-ericw-v0.15.3 

Hi, I hope I don't come across as an idiot for asking this, but what exactly am I doing wrong to get this result?

http://puu.sh/pvLvr/f08175b526.jpg

Ignore the missing textures, but how do I fix my strange lighting on the map? I'm not running with any command line options, just the map bsp.

Thanks for any help! 

it's hard to tell from that shot if it's a corrupt file, or something else.
Check for any stale .lit files and delete them.

Might be easiest to just post the .map+.bsp. 

Thanks for the suggestion about the .lit file, I found one and deleted it and now everything is working fine! :D