-
Notifications
You must be signed in to change notification settings - Fork 64
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
Disable portal related RTX options by default #50
base: main
Are you sure you want to change the base?
Conversation
This probably would be a good thing to change, though we'd need to make sure this option is enabled in Portal RTX's config and whatnot as currently it relies on this default I think (which would disable this if it received another runtime update). Mostly just adding this comment so that fact is not forgotten later. |
|
23f3d36
to
217c5a9
Compare
|
Once you address all the feedback add yourself as contributor to "Github Contributors" in |
Scratch that suggestion. There are other hl2.exe games that don't use Portals so doing it in the config.cpp is not optimal either. |
What about
|
I just checked Portal Prelude, the default Default: rtx.conf Edit: There's this line in |
Having said that, how much of perf/memory are you improving with your proposed change in a game you are optimizing for? Because this PR will require updating rtx.confs for Portal and there are more portal related RTX Option settings to account for if we are to apply your suggestion of disabling unused ray portal related effects. |
I haven't done any tests. The description says 3x of a typical froxel grid, but I have no idea how significant it is. Ever since I read through the RtxOptions.md disabling it has become a practise for me. My whole motivation is simply disable anything that is costly yet unused. Updating the configurations of Portal and Prelude is a one time operation. I think there's no need for adding an additional bit of complexity by making it conditional. |
We discussed this and it does seem like a good idea. We want to make sure Portal RTX and Prelude stay compatible, so we'll need to update those config files first before we can take a PR that changes these defaults. We'd like to cover all of the ray/portal options in one go to minimize the number of times we need to push Steam updates. @Kamilkampfwagen-II, can you modify your PR to set all relevant ray/portal options to false by default? Would be extremely helpful if you could also provide updated config files for Portal RTX and Prelude that enable all the options you disable in your PR. If you happen to get numbers on the perf delta, that may also help speed (ha!) things along on our end, but not required. |
Most other portal related options are set to false by default, though I believe Source games have
|
So do I need to turn off any other option from the list before composing the new config for Portal and Prelude? |
Yes, I believe that's all of them. Please post the resulting config files when you have them. That's also a lot of options, so I'm leaning towards having a master enable in the PR that flips them all to the current defaults. That way, any games that need to enable portals wouldn't need to figure out the magic set of options to do so. But let's get the config files updated first since we need to do that anyway. |
My point is that |
The only option that we expect will have a measurable effect today is The reason we want to cover all of these options in one go is that there is likely some perf left on the table behind these enables. Biggest savings might be getting ray/portal logic out of the resolver entirely via shader specialization. We also found that some of the CPU ray/portal logic within the scene manager could early-out based on these enables, but we don't always do that today. Making these improvements happen starts with getting these options switched off by default, which requires updating Portal RTX + Prelude. Updating shipped games requires some work, since we need to QA anything that goes live on Steam. We'd like to do this once and include all options that may result in perf benefits down the line in the update. |
1ceb252
to
c1b1497
Compare
I did amend the commit, here are the updated configs both in ZIP and as a gist |
Thanks! We'll give the conf files a go in QA. For the PR itself, I think it looks good. One suggestion would be to create a "master" enable option that toggles all of these together. This would probably work as a macro of sorts that runs immediately during config file parsing. Is this something you'd like to take on? I'd say it's optional for this PR, since it's something we can get done on our end later, but would be nice to land it at the same time as the new defaults if possible. You should also add yourself to the list of contributors in dxvk_imgui_about.cpp. |
I don't quite get it sorry, other options are inactive if |
As an update, we're trying to get the config file changes through QA now (we didn't have enough cycles to verify this leading up to the 0.4.0 release). Once that's done we should be able to post them to Steam, then merge this PR. |
cf49eb1
to
c1b1497
Compare
REMIX-3066 for tracking |
As stated in the description, this option uses additional memory yet doesn't have any effect on games with no portals, which is the majority of them.
It should be disabled by default. Note that users will have to enable it by config if they update the Portal & Prelude binaries with the latest oss release.