.lit2 format - request for feedback

[ericw]

[Baker]

I don't have anything interesting to say except I think some of the new lighting features you've worked on are very nice.

In particular, the lighting through fence textures is very impressive! Not to say the other features aren't, but that one in particular seemed borderline impossible-ish.

[ericw]

Glad to hear, Baker

Here's a direct link for the engine diff: I'm happy with how compact it is.
https://github.com/ericwa/Quakespasm/compare/lit2

This diff isn't making use of lmextents or luxdata.

[Baker]

The diff looks really clean and looks like any GL engine could implement it rather hassle-free. Sounds like you have more ideas you are implementing here ... than 2x size lightmaps when you are done.

[mh]

I'd like to see more overbrighting range in this.

Currently the light tool itself clamps at 255, and some hotspots will saturate to white rather than retain their correct colour as a result. Even something as simple as shifting the RGB values down 1 bit and using a 4x modulation scale (instead of 2x) can be effective (yes, you lose an extra bit of precision, but software Quake only used 64 light grades so I believe it can be afforded to be lost). Alternatively store the light data in a 10/10/10/2 format. Engines that can't (or won't) support this can just re-adjust the data at load time.

I'm also curious about what perf is like with lightmap updates.

[Baker]

BLOCK_WIDTH and BLOCK_HEIGHT is doubled, right? So 4x size.

How does this affect FPS *without* a .lit2? If it reduces performance in a no .lit2 case for no reason, might be a minor issue.

[With heavy rocket firing with 8 players and 30 monsters (assume a Tronyn map ).]

[Spike]

[Baker]

The qbsp without the .lit2 would be backwards compatible though, I'm assuming, so the maps would load in other engines. (Right?)

The lightmaps are loaded on level load obviously, so if I chose I could have the non-lit2 scenario use the old block size 128 should it be an issue. i.e. block_width = lit2 ? 256 : 128 ... do a malloc, etc.

[Spike]

with the way that ericw implemented control over per-surface lightmap scales, it is the qbsp that deals with the func stuff and thus the qbsp that decides the per-surface scales. it is also the qbsp that subdivides the surfaces that the engine ultimately sees. this does NOT mean that the bsp is incompatible with other engines - in fact that only happens if you use a larger subdivide size, or bsp2. this implies that the surfaces will still be 240*240 texels max ((15+1)*(15+1) luxels)
if the lit2 has a higher lightmap density on a max-size surface, the result is a surface with more than 16 luxels on each axis - exactly like hmap2's -darkplaces thing in that the max is now 256*256.
the engine must thus be able to cope with 256*256 luxels surfaces, even if the qbsp will never require it, because the light util's higher lightmap res will require it.
if the qbsp understands both the -darkplaces argument and lightmap scales, it *should* take the lightmap scale value into consideration in order to prevent the light util from needing to generate surfaces with more than 256 luxels in any direction.
if the qbsp understands -darkplaces but not lightmap scales, the light util will need to be aware that it may need to use a lower lightmap res for large surfaces, in order to avoid going over the 256*256 limit.
the only reason I don't want to require engines to support 256*16*256*16 lightmaps (just in case) is because that is a lot of memory to have to poke in order to relight anything...

to clarify, bsps will still work in any engine that supports that bsp format (ie: bsp2 or bsp29), and has nothing to do with an engine's support for lit2. potentially even the vanilla bsps can be relit with 256 times the luxel count, without changing the bsp at all.

[ericw]

Baker, re: performance impact of increasing the engine's BLOCK_WIDTH/BLOCK_HEIGHT to 256x256, as Spike said, I would expect a bit of a speedup with static lighting (less switching of lightmap textures, so larger batch sizes), and possibly a bit of a slowdown with dynamic lighting. I can test on some low-end hardware (have an intel 82815, radeon 9250) and see if there are any problems there.

MH, re: higher bit depth per sample, Spike and I discussed it a bit but didn't come to any conclusions.
10/10/10/2 might be nice. It does add a bit of extra complexity to the format.
One thing I worry about is, if it's an optional thing for engines to implement, it's yet another variable for mappers to test (does the map look good with vanilla, highres with vanilla dynamic range, highres with high dynamic range).

[ericw]

Also, MH, the tyrutils light tool has some special clamping logic that prevents colors from blowing out to white (it scales down all 3 components so that the highest one doesn't exceed 255).
Not that that's a perfect solution, but it does work around the colours-blowing-out-to-white issue.

[mh]

[mh]

[Spike]

the reason for non-structed data was so that it would be easier to store parts of the info as bspx lumps. there's no reason you can't define some bspx extents lump to enforce that info, and it would be a shame to require repacking or extra conditions just because of that.
the other 3 sets of data may or may not be required when built into the bsp itself.

(for those that are unaware of it, bspx provides custom lumps, like 'RGBLIGHTING' to embed regular .lit files into the bsp (no header of course)).

to be honest, I'm not sure what we really want to promote, but .lit2 will be needed.

the other thing is that I wasn't sure if anyone wanted to add some flags to say that certain fields were omitted (so if nothing uses any non-0 style anyway, the styles part could be omitted entirely). I didn't suggest this myself in order to keep complexity down, but the option is there.
its also possible that a lit2 will have all shifts set to 4. this still provides deluxemaps, as well as making relighting flood random lightstyles without needing them to match the original bsp.
alternatively a flag to say the shifts should be floats instead of bitshifts or something. I dunno. floats suck, but it would allow even higher res lightmaps!... okay, bad idea, but hey.

the other thing is padding. Note that they're defined as ints, shorts, bytes, and thus need no padding (why do I get the feeling that someone is going to demand float scales instead of shifts, just because of this?).

(I really don't know what I want to do with bspx, but it would be a shame to ignore it completely - I dislike NEEDING external files, although I suppose it could just embed the entire lit2 data, optionally with all dsurface_t->lightoffset==-1 and invalid styles in a two-fingers to vanilla approach - the light data will typically be significantly larger anyway).
the other thing that might be nice is to include the info from .rtlight files somehow, instead of current methods.

but back on track, structs would be better for cpu cache than descrete arrays, yes, and might remove a few conditionals from the loader, even with the extra padding. However, I don't think it'll be that significant a saving compared to the potential size of the sample data.
it really all depends on whether anyone wants to make parts of it optional/selectable-precision or not.

See, this is why I like getting someone else to make the final decisions - its so much nicer being able to blame someone else for them all.

[mh]

@Spike: understood.

One other feature that I consider a must-have: a robust method of confirming that the .lit2 file really does belong to the .bsp it's loaded with.

With v1 .lits I typically compare the data size in the .lit with the data size in the original .bsp, and if it's an even 3x the size I accept the .lit file. False positives are still possible but highly unlikely.

I'm aware of QuakeSpasm's gamedir checking for .lits, but I think a real solution should be built-in to the format itself.

Now, the heuristic I like using will obviously break down for .lit2, although you could compare numsurfs instead and say things like "it's so unlikely for two different .bsps to have both the same name and the same number of surfs that I'm going to accept this .lit2 file with five-nines probability". But I'd like to go one step further; I'd like to see something obvious in the header, something that screams out to implementers: "Hey! Use me for checking!"

So I suggest including a "bspchecksum" in the header, which is a hash of the original BSP header.