sezero / quakespasm

QuakeSpasm -- A modern, cross-platform Quake game engine based on FitzQuake.
https://sourceforge.net/projects/quakespasm/
GNU General Public License v2.0
239 stars 95 forks source link

Add Support for Lit Water #35

Closed JosiahJack closed 2 years ago

JosiahJack commented 2 years ago

Add support for lit water. This is now my own implementation.

This supports maps compiled with -splitturb as an argument on qbsp.exe using ericw's latest compile tools. Adds new cvar r_litwater which defaults to 1 for lit (same scheme as ironwail).

Test map compiled with mix of liquids, brush models, and a few colored lights.: test_liquids.zip

Showing r_wateralpha at 1.0 here, but was tested with varying values for the separate liquids. image

sezero commented 2 years ago

You should comment out or remove Mod_PolyForUnlitSurface which is now unused.

The rest: @ericwa, please review.

sezero commented 2 years ago

OK, waiting for @ericwa's review. A review from @andrei-drexler would help too.

ericwa commented 2 years ago

I'm out of date on recent map releases, can anyone link to a released map (e.g. on quaddicted) which is compiled with lit water?

I think removing the Fitzquake render-to-texture water is OK; we still have the GL1 codepath for old hardware (r_oldwater 1) and not supporting lit water there seems reasonable.

This PR changes the appearance of water slightly, by avoiding the pixellation artifacts from rendering the water warp to a texture after warping it, e.g.

release QS, gl_texturemode 1, r_oldwater 0, start.bsp:

spasm0077

this PR, gl_texturemode 1, start.bsp:

spasm0078

I guess opinions on this will be mixed, but I would lean towards thinking the pixellated one blends in with the scene better and is part of the aesthetic / shouldn't be fixed (FWIW winquake has a similar artifact on its water.) Maybe we can emulate it by snapping the texture coords / add a cvar for people who prefer the "smoother" look?

JosiahJack commented 2 years ago

I'm going to work on this one a bit as I'm not a fan of some things.

Goals: Preserve r_oldwater 1 behavior (currently fails) Preserve current look as an option (currently fails) Preserve current look with simple lit water addition (currently fails) Provide shader based warp with lit water (ok if slightly different) (pass)

Not having much luck preserving the pixel shift effect by modifying the shader, but will keep at it.

Novum commented 2 years ago

No idea why litwater support needs to change the way newwater is rendered. The vkQuake patch for this was pretty small, why does this not work for Quakespasm mostly the same?

sezero commented 2 years ago

No idea why litwater support needs to change the way newwater is rendered. The vkQuake patch for this was pretty small, why does this not work for Quakespasm mostly the same?

If I followed the git logs correctly, the commits would be https://github.com/Novum/vkQuake/commit/7b2e0e70bb004998831271308ef89ad27eb17826 and https://github.com/Novum/vkQuake/commit/3f5f449055e2bc8cd2e7e0ab52cf3b70e1a077a7 (ref issues https://github.com/Novum/vkQuake/issues/413 and https://github.com/Novum/vkQuake/issues/449), yes?

Novum commented 2 years ago

Seems about right

JosiahJack commented 2 years ago

This has now been updated to use only my own implementation without changing the way warpimage is done. Tested and still falls back to unlit for maps that weren't compiled with splitturb. Works for the various alpha settings for each liquid type and does not appear to affect alpha sorting.

sezero commented 2 years ago

OK. @ericwa, @andrei-drexler, @Novum: any objections?

ericwa commented 2 years ago

I've only found one bug: r_oldwater 1 on test_liquids.bsp. spasm0000 start.bsp water looks fine so it's the combination of lit water in the bsp + r_oldwater 1.

ericwa commented 2 years ago

I think this fixes it.. the problem was due to calling Mod_PolyForUnlitSurface, then GL_SubdivideSurface, then BuildSurfaceDisplayList on the same surface.

diff --git a/Quake/gl_model.c b/Quake/gl_model.c
index 0f0eb984..8429bbcd 100644
--- a/Quake/gl_model.c
+++ b/Quake/gl_model.c
@@ -1329,8 +1329,13 @@ static void Mod_LoadFaces (lump_t *l, qboolean bsp2)
                out->flags |= SURF_DRAWTELE;
            else out->flags |= SURF_DRAWWATER;

-           Mod_PolyForUnlitSurface (out);
-           GL_SubdivideSurface (out);
+       // polys are only created for unlit water here.
+       // lit water is handled in BuildSurfaceDisplayList
+           if (out->flags & SURF_DRAWTILED)
+           {
+               Mod_PolyForUnlitSurface (out);
+               GL_SubdivideSurface (out);
+           }
        }
        else if (out->texinfo->texture->name[0] == '{') // ericw -- fence textures
        {
diff --git a/Quake/r_brush.c b/Quake/r_brush.c
index a2edf57f..0ee701b9 100644
--- a/Quake/r_brush.c
+++ b/Quake/r_brush.c
@@ -891,6 +891,10 @@ void BuildSurfaceDisplayList (msurface_t *fa)
    //johnfitz -- removed gl_keeptjunctions code

    poly->numverts = lnumverts;
+
+   // support r_oldwater 1 on lit water
+   if (fa->flags & SURF_DRAWTURB)
+       GL_SubdivideSurface (fa);
 }

 /*
ericwa commented 2 years ago

With my fix applied I'm fine with this being merged!

JosiahJack commented 2 years ago

Fixed r_oldwater 1 rendering by applying ericw's fix.

sezero commented 2 years ago

This is in - thanks.

sezero commented 2 years ago

@ericwa, @andrei-drexler, @Novum: send patches if issues are discovered in future.

sezero commented 2 years ago

qs-0.95.0 is released with this feature included.