Sun doesn't contribute to scene lighting

[radgeRayden]

Built from the latest commit of the brushbsp branch. Any form of sun contribution seems to be ignored in the lightmapping process. From elimination the bug manifests from the new qbsp executable. Using the latest vis and light with the 0.18.2 qbsp executable gives the expected correct result. I have not determined the earliest commit where the bug appears. Attached are two screenshots, with the expected result and what I get with the latest build. Test map also attached (only depends on START.WAD).


sun_test.zip

I have tested and verified that the bug occurs both on Windows 10 and Arch Linux.

[fabiolimamp]

Can confirm the bug happening in the current 2.0.-alpha1 build. But I in my case I tried building the BSP with 0.18.2 qbsp and still got no sun.

Another thing is that the sun doesn't render either when set in worldspawn or via a light entity.

[radgeRayden]

Admittedly my testing wasn't extensive, so I probably made a mistake. @fabiolimamp can you confirm that qbsp is the only affected executable?

[fabiolimamp]

Admittedly my testing wasn't extensive, so I probably made a mistake. @fabiolimamp can you confirm that qbsp is the only affected executable?

Nope, like I said, even running the BSP step with the 0.18.2 qbsp,exe got no sun for me.
All I had in terms of settings was a wadpath set and the bsp2 switch.

[dsvensson]

Same here, going back to a5c0f0e brings the light back, haven't checked what commit closer to HEAD that still works. Oddly enough lightpreview is not affected. Fun thought would be if CI would generate a huge atlas and upload somewhere for before/after CI build for a set of variations.

[ericwa]

f93a36c is what introduced the breakage. I don't understand why because that commit was only supposed to change the handling of missing textures to match the old behaviour, but I'm compiling sun_test.map with all textures available so the handling of missing textures shouldn't be relevant..

[Paril]

Are we removing texture names from dummy textures? They should still be retained...

[ericwa]

I must just be corrupting the dmiptex lump in that commit..

Are we removing texture names from dummy textures? They should still be retained...

I think we need to remove them for backwards compatibility; prior to f93a36c, maps with missing textures would crash winquake, and other engines. vkQuake + QS and others applied a patch to work around this, because some bsp's were released in a map pack with these 0x0 textures:

Novum/vkQuake#567

They also rendered wrong in Fitzquake/QS - missing textures are supposed to render with an orange checkerboard.

Also I think I broke -notex in f93a36c . -notex is documented as producing backwards-incompatible bsp's so I think that should be our control for writing 0x0 textures maybe? I'll need to double check what -notex did in the release version.

[ericwa]

I think I finally tracked down what's happening here.

The bug is suppressed if you have a .wad loaded containing a "skip" texture (and no other missing textures in the map.)

If you don't have that, the "skip" texture we always write in qbsp gets written as -1 and with no name.
Then in light, we register it at some point as a texture called "" (empty string).
Then this code in AddSun does a img::find("") which returns an actual texture (the "skip" placeholder), and accidentally activates the "filter sunlight to only emit from specific textures" codepath, which is what hides the sunlight:

sun.suntexture = suntexture;
sun.suntexture_value = img::find(suntexture);

[Paril]

Oh this explains why we had crashes in the _project_texture pathways too; it was finding an empty image with no pixels, and then divided by zero...