[x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Issue

[Batandy]

Hi, i'm very glad you addressed the dynamic lights issue i've posted some days ago, but i've noticed there still is a problem, the lights seem to have boosted values?

Again, i'm testing this with my Castlevania mod ( http://www.mediafire.com/file/9l5h70va0 ... y_v1.2.zip )


This is how the first room of the game is supposed to look like


This is how it looks in the latest dev build, notice how bright everything is compared to the other pic, every dynamic light is almost double its intended size

[_mental_]

The issue was introduced with removal of these lines.
Dynamic lights should be attenuated by default, right? Now the corresponding flag is not used at all.
It seems those lines were removed accidentally. Comments are misleading but code itself looks correct.

[Graf Zahl]

The removal of these lines was deliberate. With the render backend unification it is not possible to change the backing data. This needs to be done when the lights are processes.
What this report lacks is the usual: What's the hardware this happens on? It has to be OpenGL 2.x, I think. I actually thought that I made the necessary adjustments to the renderer but this looks like that doesn't work.

[_mental_]

This happens on Windows (OpenGL 4.6 NVIDIA hardware, with and without -glversion 2.0) and macOS (OpenGL 4.1 and 2.1, Intel).
After this change FLightDefaults::GetAttenuate() is no longer used. That's why dynamic lights radii are bigger than before.

[Graf Zahl]

That code should never have run for GL 3+. Its entire purpose is to ensure that attenuated lights do not become too big on GL2 - and that logic was wrong to begin with. The error must lie somewhere else. Apparently something else got deleted as well.

[_mental_]

I'm using 3.3.2 as a reference. Loading Castlevania triggers this condition 19 times.
Actually, the given code runs regardless of render settings during parsing of GLDEFS.

[Graf Zahl]

You are only looking at the symptom of the problem, not its real cause. The code should not run under GL 3+. I can't check right now when the Renderer->CanAttenuate call got commented out, but that was there to only make this check on legacy GL - but the conditions for that no longer apply with the software renderer also present.
If this got somehow activated before the 2D_Refactor branch something went very wrong and needs to be investigated.

I can give you more info later this evening. I'm at work right now and cannot test

[_mental_]

I have no idea how it should work indeed. All I know, it's being like that for a while.

[Graf Zahl]

Shit!

As it is very obvious, that commit forgot to finalize its changes. The CanAttenuate function was simply forgotten.
So this means we have a gigantic problem now with some light definitions using proper sizes and others using the bad ones starting from that commit.

The code as it is now is the correct version, like it should have been - and like it had been working before that change, where the light shrinking was only done for GL 2.

[_mental_]

What should we do then? With exception of leaving everything as is, a compatibility setting can be added to handle this.
It will be solely on user's discretion to enable it though. I see no means to detect oversized light definitions reliably.

[Rachael]

Well - considering the light shrinking has been around in official releases since attenuated lights were first introduced (AFAIK no release ever had it without the shrinking)... wouldn't it make sense to have them work as much as possible "how they used to" from an end-user's point of view?

[Graf Zahl]

Well, every single time that a bug was retained because 'that's how things worked for too long' in the end the problems far outweighed the benefits.
Additionally, all the internal lights have been done with the proper handling.

So no, I will not keep this stuff in. What we can do is to provide an automatic compatibility handling based on a lump's hash and an optional per-lump scaling factor, so that fixing a broken setup amounts to only adding one line to the definitions.
Moreover, the bug manifests itself only as a bug on GL3 and GL4, but on GL2 worked as intended. So we cannot even assume that anything made over the last year wants this.

[Rachael]

A whole year of stuff is a lot to account for... especially given that this is one of the more active development years considering dpJudas's numerous contributions over that time span, which I am sure has spurred a lot of work on mod author's sides.

[Graf Zahl]

One year or not, I am not willing to keep the broken setting as a default. I'll do what I can to keep this stuff working, but as it stands, the lights worked differently on GL2 and GL3 so in many cases it's hard to tell what was wanted.

But like I said, every time when it was decided to keep the broken way as a default, the problems just kept piling up. You can go back to 1998 and see many of such things. It was the same story repeated all over each time. And the longer the bug was retained the harder it got in the end to sanitize it. No, thank you!

I'm on vacation for the next 4 days but I'll see that I can get something working this evening to help mitigate the issue without retaining the bug. That said, I think we really need a MAPINFO versioning system for the future so that mods that target a version with some broken feature can be auto-corrected later, if things need to change.

[Rachael]

Agreed.