[0.6.0-452-gfc466849ce] Lighting isn't diminishing properly

[mjr4077au]

Sounds very exciting . I'll be keen to see that stuff as it develops as time goes on. Almost home now so I'll test that code out soon.

[mjr4077au]

So, the PR I drafted and sent through provides consistent lighting for me on the work laptop and my desktop, both of which are different resolutions. I don't have any answer for why lowskill. or yourself couldn't notice a difference, but it is there.

PR #78 is the draft commit with the magic number constant replaced with the algorithm used to obtain it. It's also fixes the issue with the least amount of changes.


PR #79 is an alternate take on fixing the issue. It changes the code a lot more into a process that is more fidele to the code before 0bd460d9e3bbaf239f4a33f9ce589dff58660bea, but probably done in a nicer/cleaner way. From my review, the 'h' and 'c' letters in the old globals correlated to 'floor' and 'ceiling' respectively, at least to my understanding, except for one exception, because Build...


These are not the same images, each commit produces an identical end-user experience. One question I do have, if setting the mGlobVis by renderSetVisibility(), are all the GLInterface.SetVisibility() calls actually necessary? I haven't tested the overall effect of stripping them out. EDIT: They're needed .

I'd probably go with the first PR as it's less OpenGL-focused and fixes it more in-line with the original intent of 0bd460d9e3bbaf239f4a33f9ce589dff58660bea.

[Graf Zahl]

I'd prefer #78. Whatever voodoo this code was supposed to do I really don't want to know - does it mean that the fade gradients are different for floor, ceiling and walls?

[mjr4077au]

In the code, it would appear so. In practice, from my limited testing, I couldn't tell the difference. I'd need to play each game more to confirm. This spot in Duke E1L1 has just been my go to test point due to the step fall off of lighting.

Just saw that #78 got merged which I think was the right move. Considering I was happy with the original fix and #78 just makes it consistent between resolutions, it should be all that's needed

[sinisterseed]

Well, it's good that now it has finally been fixed, or at the very least it addresses the most immediate concern, that this now became resolution dependent.

As for Polymost, the plan all along was to fix only what was really necessary and then for other things and new features to build a new renderer. The problem with Polymost is that actually fixing it fully is just way, way more work than it warrants, it's too poorly written and inflexible as it can be seen with this matter at hand. And from my conversations with Graf on this matter, I understand that it's also not exactly in a state where it could be properly and fully refactored, unlike, say, the GL render in the Doom port PrBoom, which is actually what GZDoom's render eventually emerged from decades ago.

While Polymer's issues are different, it shares the same fate due to the way it was written, and it was very inefficient. It was sweet and pretty, and in terms of visuals it was definitely my personal favorite GL render in EDuke32, but its flaws prevented it from replacing Polymost, which is basically what it was meant to do from what I gather, but it was never fully finished and the guy who wrote it has moved from it very long ago to NVIDIA. It was given to the eduke guys but essentially saw no further development. It simply isn't worth investing intense effort into these two renderers, it's much better and also easier to build something else entirely that is more performant, feature rich, flexible, and without any of the drawbacks and design flaws or limitations that these two had.

[Phredreeke]

Valve, Plagman moved on to Valve.

[Gez]

In the code, it would appear so. In practice, from my limited testing, I couldn't tell the difference. I'd need to play each game more to confirm. This spot in Duke E1L1 has just been my go to test point due to the step fall off of lighting.

Replacing floor, ceiling, and wall textures by one that's solid white would make it easier to check if there are differences in how gray pixels become at distance X depending on which surface they're on.

[mjr4077au]

Replacing floor, ceiling, and wall textures by one that's solid white would make it easier to check if there are differences in how gray pixels become at distance X depending on which surface they're on.

I'll have a look and try this when I get home. Should just be able to turn off textures with a CVAR I believe.

[PlayerLin]

To the EDuke32 devs, Polymer just a mess to them(and they already gave up long, long ago as nobody can finger it out), same thing to Graf sees the Polymost.

Sad.

[mjr4077au]

Replacing floor, ceiling, and wall textures by one that's solid white would make it easier to check if there are differences in how gray pixels become at distance X depending on which surface they're on.

I'll have a look and try this when I get home. Should just be able to turn off textures with a CVAR I believe.

Found there was no CVAR, created PR #82 to add one in the same fashion as how GZDoom has so I can get this testing done.

From some quick playing, irrespective of whether PR #78 or PR #79 is used, there is still subtle lighting differences compared to 0.6.0 that I don't know whether it's worth tracking down or not. Take the below example of the spaceship crashing in Duke3D E1L1.

From 0.6.0:


From current master:


You can see, the explosion is less visible after all the backend changes. Subtle, but noticable nevertheless.

[sinisterseed]

So it's now darker than before, if ever so slightly, as if the first time around it was fullbright but now it isn't.

I dunno what to say tbh, this might be worth chasing down if, for whatever reason this may cause other issues too, but I'm not sure this is caused by the sector lighting now, this could be another change.

[mjr4077au]

So it's now darker than before, if ever so slightly, as if the first time around it was fullbright but now it isn't.

I dunno what to say tbh, this might be worth chasing down if, for whatever reason this may cause other issues too, but I'm not sure this is caused by the sector lighting now, this could be another change.

It's likely the removal of these lines and these lines. I'll see about testing it to find out.

EDIT: Wasn't it and not sure where to look further. I'd have to build the commit before the aforementioned one to see whether its present or not. I'm not going to do anything further until Graf says whether he wants effort spent on it or not to bother.

[Graf Zahl]

If you want to have your try, go for it. In the end we hopefully learn a bit about Polymost here that might be useful later.

[mjr4077au]

Unexpected given all the other backend work, but dropping 9b03537f3a235c3146b689892e9da145f4a1d0c1 resolves that lighting depth issue that I've reported. Unsure why, but it's the cuprit for sure.

Another instance where the lighting depth has issues is in my test spot on Duke 3D in E1L1.

0.6.0:


Current Master


Note the brightness of the cinema in the corner of the window. Dropping the above commit fixes that as well. The missing glow on the graffiti in 0.6.0 was broken by something else and is not reverted by reverting the aforementioned commit.

[Graf Zahl]

Interesting. It seems the engine uses negative shades to mess around with its lighting. So the lower clamp to 0 needs to go, but the upper clamp to numshades needs to remain - without that we get undefined behavior in the renderer.