GraphicsSettings: EFBAccessEnable=false by default
Makes "Graphics -> Hacks -> Skip EFB Access from CPU enabled by default.
Due to some drivers resulting in GPU stalls when EFB access occurs in games where EFB is not even used. Most games that require this setting set to 'true' (thus being disabled) already have this defined in their game inis.
I am in favor of this change. Most forks of Dolphin already do this anyway and we know how much users love those.
I'm in favour of this change, however, by just flipping the bool in the code, this will only be applied to fresh Dolphin installs, existing users will have this turned off.
In the past we handled that by changing the INI key, perhaps we should do that too? It would also be a perfect opportunity to make the true/false value match the checked/unchecked status in the GUI...
I mean, I don't mind it not overriding old defaults. Not saying it has to be this way, but it is also the simplest way to go about it.
I'm in favour of this change, however, by just flipping the bool in the code, this will only be applied to fresh Dolphin installs, existing users will have this turned off.
If the user has never touched the setting, the new default will apply. Note that this was different in the past before we switched to the layered config system.
You must also update the default value in the Android GUI: https://github.com/dolphin-emu/dolphin/blob/da530a2eb9d217821a0afcfb0478b62a2672112f/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/BooleanSetting.kt#L792
You must also update the default value in the Android GUI:
https://github.com/dolphin-emu/dolphin/blob/da530a2eb9d217821a0afcfb0478b62a2672112f/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/BooleanSetting.kt#L792
Good catch, thanks.
Any further thoughts on this? I think this is a slam dunk swap.
If we want this to be on by default, should we maybe also update the description to say "If unsure, leave this checked"?
Personally I'm leaning towards not renaming the INI key, especially to not break existing game configs.
maybe also update the description to say "If unsure, leave this checked"?
Good catch, the tooltip should be updated
I'm leaning towards not renaming the INI key, especially to not break existing game configs.
Exceptionally in this case, that's desired, IMO
...update the description to say "If unsure, leave this checked"?
Done; Also installed Android and checked there were no such tooltips etc needing updating.
There are 4 .ini files that specify EFBAccessEnable = false:
- GT6 (Terminator 3: The Redemption)
- GXB (SSX3)
- RTH (Tony Hawk's Downhill Jam)
- SNC (SONIC COLOURS)
Most of the commits that added those settings were many years ago, so I don't know if they're specifically needed anymore.
There are 4 .ini files that specify EFBAccessEnable = false:
* GT6 (Terminator 3: The Redemption) * GXB (SSX3) * RTH (Tony Hawk's Downhill Jam) * SNC (SONIC COLOURS)Most of the commits that added those settings were many years ago, so I don't know if they're specifically needed anymore.
I'm not sure why they would be set to =false unless they were also bringing up the GPU hanging issue. It doesn't really make sense for that to be in the game inis.
Do we want to remove them in this PR, now that this is the default anyway?
Sure, why not remove them.
I checked the history of the Sonic Colors INI, and we've had EFB access disabled for that game ever since the game INI system was added in 2009, without any explanation as far as I was able to find.
Personally I'm against renaming the INI key, for what it's worth, because it would introduce a break for any user INIs that have forced the value one way or another. For sys INIs we could account for that, but not for user ones.
Sure, why not remove them.
Does it make sense to delete a Sys\GameSettings ini entirely if there were no other changes?
Example: GXB.ini
# GXBE69, GXBP69 - SSX3
[Core]
# Values set here will override the main Dolphin settings.
[OnFrame]
# Add memory patches to be applied every frame here.
[ActionReplay]
# Add action replay cheats here.
[Video_Hacks]
EFBAccessEnable = False
Or should I just remove the Video_Hacks section entirely, but keeping the empty ini? I checked and other inis omit the section if not used.
INIs like this, that only contain sections and comments without any actual settings, should be deleted.
Pushed, wasn't sure if it made sense to have that in the same commit; Let me know if it should be squashed.
Let's try this out.