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

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

Forum rules
Please don't bump threads here if you have a problem - it will often be forgotten about if you do. Instead, make a new thread here.
User avatar
Batandy
Posts: 1277
Joined: Tue Jul 19, 2011 2:56 am

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

Post by 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 )

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

Image
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_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by _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.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by 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_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by _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.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by 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_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by _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.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by 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_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by _mental_ »

I have no idea how it should work indeed. All I know, it's being like that for a while.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by 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_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by _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.
User avatar
Rachael
Posts: 13530
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by 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?
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by 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.
User avatar
Rachael
Posts: 13530
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by 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.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by 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.
User avatar
Rachael
Posts: 13530
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: [x64-g3.4pre-236-gc1ce6c90ca] Another Dynamic Lights Iss

Post by Rachael »

Agreed.
Post Reply

Return to “Closed Bugs [GZDoom]”