DarkPlacesEngine / DarkPlaces

The DarkPlaces Quake engine, created by LadyHavoc. Official Git repository (replaces SVN).
https://icculus.org/twilight/darkplaces/
GNU General Public License v2.0
273 stars 39 forks source link

Textures in a fullbright map (a Q1 map with no light data). #103

Open Baker7 opened 11 months ago

Baker7 commented 11 months ago

This one should be easy to fix. I'll post a fix when the opportunity presents.

dpbeta_xon_undergate

dpbeta_undergate

If I type "r_fullbright 1" ... it renders correctly.

Map download: https://www.moddb.com/mods/darkplaces-classic-set/addons/the-undergate (-game undergate +map undergate)

Baker7 commented 11 months ago

This does fix it, although I'm not quite comfortable with it.

The rendering in DP master is quite different than Xonotic 0.8.6 / old DarkPlaces and so my comfort level of this is less because some of the rendering works differently.

What the problem is:

        // force lightmap upload on first time seeing the surface
        //
        // additionally this is used by the later code to see if a
        // lightmap is needed on this surface (rather than duplicating the
        // logic above)
        loadmodel->brushq1.lightmapupdateflags[surfacenum] = true;
        loadmodel->lit = true;

This is in Mod_Q1BSP_LoadFaces after we loaded the lighting. loadmodel->lit = true; <---- well, not necessarily

loadmodel->lit = loadmodel->brushq1.lightdata ? true : false; // Baker: Set lit depending on whether we have lightdata

Then what works ... these are mostly in cl_main.c and clvm_cmds.c and csprogs.c is hitting every r_fullbright.integer and adding a check for light data.

Before: if(!r_fullbright.integer)
After:  if(!r_fullbright.integer /*0*/ && r_refdef.scene.worldmodel && r_refdef.scene.worldmodel->lit)
Before: if (!(ent->flags & RENDER_LIGHT) || r_fullbright.integer)
After:  if (!(ent->flags & RENDER_LIGHT) || (r_fullbright.integer || !r_refdef.scene.worldmodel || !r_refdef.scene.worldmodel->lit) )

Is this the "right" way to do it? I can't say. The old codebase used R_LightPoint to do this check and set ambient/diffuse/etc to 1 if no lightdata.

hemebond commented 11 months ago

Can you share the source for the map?

Baker7 commented 11 months ago

I lost it ages ago, but the map decompiles easy with any q1bsp tool. (I forgot to put .map source in it on release, I was so focused on the .qc source). The map was released as gpl 2.

The map source is nothing special. Several cube entities with teleporters if they rise.

hemebond commented 3 days ago

I don't even have lit floor (maybe that's an external texture in your screenshot). This is definitely different behaviour to QuakeSpasm engines.

Proper thing to do would be to compile the map with minlight, but would be good to match other engines.

hemebond commented 3 days ago

@Baker7 Please have a look at PR #206 and let me know what you think.

Baker7 commented 3 days ago

@Baker7 Please have a look at PR #206 and let me know what you think.

I'll need to refresh my memory on this by looking at the DarkPlaces classic source code. Zircon has this issue fixed, I just want to revisit looking at the old source code to make sure what I did would still be what I would do if I were to do it today.

Baker7 commented 3 days ago

In Zircon engine, these are the changes:

cl_main.c(1440): if (!r_fullbright.integer /0/ && r_refdef.scene.worldmodel && r_refdef.scene.worldmodel->lit) { // Baker r1002: Proper Quake behavior for Q1BSP maps with no light data -- all entities in map render fullbright. cl_main.c(1898): // Baker r1002: Proper Quake behavior for Q1BSP maps with no light data -- all entities in map render fullbright. cl_main.c(1931): // Baker r1002: Proper Quake behavior for Q1BSP maps with no light data -- all entities in map render fullbright. cl_main.c(2922): if (!(ent->crflags & RENDER_LIGHT) || (r_fullbright.integer || !r_refdef.scene.worldmodel || !r_refdef.scene.worldmodel->lit) ) // Baker r1002: Proper Quake behavior for Q1BSP maps with no light data -- all entities in map render fullbright. clvm_cmds.c(2637): if (!r_fullbright.integer && r_refdef.scene.worldmodel && r_refdef.scene.worldmodel->lit) { // Baker r1002: Proper Quake behavior for Q1BSP maps with no light data -- all entities in map render fullbright. csprogs.c(414): if (!r_fullbright.integer && r_refdef.scene.worldmodel && r_refdef.scene.worldmodel->lit) { // Baker r1002: Proper Quake behavior for Q1BSP maps with no light data -- all entities in map render fullbright. model_brush.c(2717): loadmodel->lit = loadmodel->brushq1.lightdata ? true : false; // Baker r1002: Proper Quake behavior for Q1BSP maps with no light data -- all entities in map render fullbright.

6 changes which are marked with "r1002" in 4 files. A quick glance are your modification, and I only see 2 changed files.

In model_brush.c, DarkPlaces Beta did not fill in loadmodel->lit during map load.

During each of the 4 checks in cl_main.c, it is checking against r_refdef.scene.worldmodel->lit

Your change, it looks like you are checking if individual entities are lit, instead of checking if the world is lit (does the world have lightdata? yes or no).