Skip to content
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

Fix deferred lighting Option #6346

Merged
merged 5 commits into from
Sep 9, 2024
Merged

Conversation

BMagnu
Copy link
Member

@BMagnu BMagnu commented Sep 9, 2024

Similar issue to #5966.
This now provides a way to ask the option for its value instead of relying on a side effect assignment.
At the same time, this is now cached for set-once options, allowing for fast querying.
Furthermore, it gets rid of some really really ugly code throwing around raw pointers and kind of illegally managing memory in favor of shared_ptr's, as there's a good chance that that was part of why the compiler mis-identified the optimization in the first place.

Should fix #6344

This should probably be tested in release mode by someone with an MSVC compiler.

To be frank, this is a hotfix only.
A more permanent refactor of the ingame options will be needed for 25.0 to a) optimize lookup performance, while b) not relying on side-effect access to variables that the compiler can mess with. Options should store their state fully internally, not giving a user access to the data outside of the option object.

@BMagnu BMagnu added the fix A fix for bugs, not-a-bugs, and/or regressions. label Sep 9, 2024
@BMagnu BMagnu added this to the Release 24.2 milestone Sep 9, 2024
Copy link
Member

@wookieejedi wookieejedi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With these most recent commits now works in my tests.

@BMagnu BMagnu enabled auto-merge (squash) September 9, 2024 21:28
@BMagnu BMagnu merged commit b6879da into scp-fs2open:master Sep 9, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A fix for bugs, not-a-bugs, and/or regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential performance regression in 24.2 cycle
2 participants