thats an old version, only watervising added to it.

https://www.quaddicted.com/files/tools/wqbsp165.zip 

According to the WQBSP165 docs, it doesn't support -splitspecial, so lit water won't work with it.

The problematic entity is a light torch; I've tried deleting it and creating a new one at the exact same place, and the problem still happens. It's like the tyrutils qbsp compiler is too strict about entity placement.

Light entities should be treated as point entities by the QBSP compiler. I wonder if the tyrutils qbsp is checking whether some default bounding box size is out of map bounds or something. 

weird, can't reproduce here.
I have a START.MAP modified 4 Sep 1996. No leaks with qbsp.exe from tyrutils-ericw-v0.15.4-115-gfff8cbf. Adding -splitspecial doesn't cause leaks, either. 

o_O

rebb? eric? carmack? any ideas? 

Anyway, I've confirmed that my code for lit water wasn't implemented. -lightturb doesn't make any difference.

See the result with tyrutils-ericw-v0.15.4-115-gfff8cbf: scr_0778.png

And the proper result with my old modified build: scr_0779_mankrip.png

So, there's still no way to compile maps with both lit water and smooth shading. :( Oh well. 

I forgot about http://forums.insideqc.com/viewtopic.php?f=12&t=5769

But I'm confused.. now I realize that -splitspecial means water won't have TEX_SPECIAL set. This means lit water should "just work" without changes to light - so the -transwater stuff Spike added is unnecessary. Your patch just makes the bottom face receive light from above, rather than underwater, right? That seems sensible since typically most of the light sources will be above the water, but not having that shouldn't cause the artifacts in your first screenshot. 

all I can say is that the water parts of my patch were completely untested, and only half written. yeah, I got bored and then forgot about it, okay? please forgive my sins! so that I can more easily commit them in future! mwahaha!

the bright part of the water is probably a poly with no lightmap info at all (which technically means all-white-but-not-overbright instead of all-black when found on water surfaces).

this would imply that much of that area is normally lit from below.

I don't think it tries to subdivide water either. I guess this means that the controls for whether water should be lit should really be inside the qbsp instead. 

Your patch just makes the bottom face receive light from above, rather than underwater, right?

It makes each face receive light from both under and over the water, to match the lighting on the nearby walls. The normal inversion is done on a per light entity basis - the face's normals will only be inverted for light entities behind it.

now I realize that -splitspecial means water won't have TEX_SPECIAL set. This means lit water should "just work" without changes to light

-splitspecial also splits the water surfaces into the appropriate size for lightmap creation.

typically most of the light sources will be above the water, but not having that shouldn't cause the artifacts in your first screenshot.

As mentioned at InsideQC, my code includes a hack to force liquid surfaces' lightmaps to always be saved, otherwise fully dark liquids would remain fullbright. 

It would be better if the -splitspecial option in the qbsp was replaced with -splitturb and -splitsky options. There's no need to split sky surfaces, because there's no need to make them receive regular lighting. 

It makes each face receive light from both under and over the water, to match the lighting on the nearby walls. The normal inversion is done on a per light entity basis - the face's normals will only be inverted for light entities behind it.
Ok, cool, I'll try merging this in.

As mentioned at InsideQC, my code includes a hack to force liquid surfaces' lightmaps to always be saved, otherwise fully dark liquids would remain fullbright.
I think this bit is missing from your patch?
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? 

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.