Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Baker7 opened this issue Oct 12, 2023 · 7 comments · May be fixed by #206
Open

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

Baker7 opened this issue Oct 12, 2023 · 7 comments · May be fixed by #206
Labels
bug Something isn't working regression Something used to work in an earlier version but isn't working anymore

Comments

@Baker7
Copy link

Baker7 commented Oct 12, 2023

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
Copy link
Author

Baker7 commented Oct 13, 2023

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
Copy link
Contributor

Can you share the source for the map?

@Baker7
Copy link
Author

Baker7 commented Oct 13, 2023

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 hemebond added the bug Something isn't working label Sep 25, 2024
@hemebond
Copy link
Contributor

hemebond commented Sep 25, 2024

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 hemebond added the regression Something used to work in an earlier version but isn't working anymore label Sep 25, 2024
@hemebond hemebond linked a pull request Sep 25, 2024 that will close this issue
@hemebond
Copy link
Contributor

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

@Baker7
Copy link
Author

Baker7 commented Sep 25, 2024

@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
Copy link
Author

Baker7 commented Sep 25, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression Something used to work in an earlier version but isn't working anymore
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants