Skip to content

Nuking the obsolete deprecated legacy forward renderer? #747

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

Closed
illwieckz opened this issue Nov 19, 2022 · 5 comments · Fixed by #1670
Closed

Nuking the obsolete deprecated legacy forward renderer? #747

illwieckz opened this issue Nov 19, 2022 · 5 comments · Fixed by #1670
Labels
A-Renderer T-NUKE-Request NUKE - vt. to remove a feature with extreme prejudice T-Question

Comments

@illwieckz
Copy link
Member

illwieckz commented Nov 19, 2022

I think I'm going to nuke the obsolete dynamic lighting renderer. It doesn't work anymore on master (no dynamic light rendered) and the only reason why it is kept is that it has some features not ported to the tiled renderer.

But those features are either broken (dynamic shadows) or unused so maybe broken (no one knows how to use them).

For now I kept them around because it was an interesting reference implementation someone could borrow interesting things when re-implementing those features on the tiled renderer.

The legacy forward renderer is considered as deprecated since 2 years: be7229a

But no one tried to port the features in those two years, and unless there is an hidden skill in some of us right now no one of the active people has the skill to do such port.

Nuking this code will nuke thousands of lines of code.

What's your opinion?

@illwieckz illwieckz changed the title Nuking the obsolete deprecated legacy forward renderer? Nuking the obsolete deprecated legacy forward renderer? Nov 19, 2022
@DolceTriade
Copy link
Contributor

I was wondering why dyanmic lighting looked so weird when I've been playing to the point I'd rather have it be disabled all together :(

I wish I could use the old forward renderer 😄

@necessarily-equal
Copy link
Contributor

I don't really have an opinion on this since

  1. I have never seen it working;
  2. I don't know that part of the code so I don't know how useful or useless this can be as a reference implementation to be ported;
  3. I don't know how much of a maintenance pain it can be.

I think if you want to nuke it it probably means it's not worth keeping it around as you try hard to keep legacy code.

In fact I thought the legacy renderer was already dropped.

@slipher slipher added the T-NUKE-Request NUKE - vt. to remove a feature with extreme prejudice label Dec 7, 2023
@illwieckz
Copy link
Member Author

illwieckz commented Dec 7, 2023

To remove confusions:

In fact I thought the legacy renderer was already dropped.

There was two things:

  • The Legacy OpenGL 1 renderer
  • The Modern OpenGL 3 renderer

Those were even two separate code trees, two separate libraries. We dropped the first one a decade ago.

Then, in the remaining Modern OpenGL 3 renderer, there is the part that is used to render lights and things like that, it's more a subset of the whole “Modern OpenGL 3 renderer”, it's a set of functions and a different code path.

So let's say we have two code paths in the “Modern OpenGL 3 renderer” and we may NUKE one of them.

@VReaperV
Copy link
Contributor

VReaperV commented Nov 1, 2024

This can be done once the required features from #1209 are implemented.

@VReaperV
Copy link
Contributor

From an earlier IRC discussion: this also requires adding CPU dynamic light culling code for devices that cannot run the lighttile code due to ALU limitations. Possibly also for hardware lacking certain required extensions, in which case we can have a macro bypassing the tile fetching code, using a small array of dynamic lights instead.

Additionally, instead of culling the lights we can set some of them as high-priority, e. g. indicators on maps or flashlights (if we had them), and then only process those.

slipher added a commit to slipher/Daemon that referenced this issue May 20, 2025
It still kinda worked but we don't want to maintain it.
Fixes DaemonEngine#747.
slipher added a commit to slipher/Daemon that referenced this issue May 21, 2025
It still kinda worked but we don't want to maintain it.
Fixes DaemonEngine#747.
slipher added a commit to slipher/Daemon that referenced this issue May 22, 2025
It still kinda worked but we don't want to maintain it.
Fixes DaemonEngine#747.
slipher added a commit that referenced this issue May 22, 2025
It still kinda worked but we don't want to maintain it.
Fixes #747.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Renderer T-NUKE-Request NUKE - vt. to remove a feature with extreme prejudice T-Question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants