Let's reset m_last_used for each register that will be used
in an instruction before we start allocating any of them,
so that one of the earlier allocations doesn't spill a
register that we want in a later allocation. (We must still
also increment/reset m_last_used in R and RW, otherwise we
end up in trouble when emulating lmw/stmw since those access
more guest registers than there are available host registers.)
This should ensure that the asserts added earlier in this
pull request are never triggered.
If the register pressure is high when allocating registers,
Arm64FPRCache may spill a guest register which we are going to
allocate later during the current instruction, which has the
side effect of turning it into double precision. This will have
bad consequences if we are assuming that it is single precision,
so let's add some asserts to detect if that ever happens.
PR #2663 added a Jit64 implementation of dcbX and a fast path to skip JIT cache invalidation. Unfortunately, there is a mismatch between address spaces in this optimization. It tests the effective address (with the top 3 bits cleared) against the valid block bitset which is intended to be indexed by physical address. While this works in the common case, it fails (for example) when the effective address is in the 7E... region (a.k.a. "fake VMEM"). This may also fall apart under more complex memory mapping scenarios requiring full MMU emulation.
The good news is that even without this fast path, the underlying call to JitInterface::InvalidateICache() still does very little work in the common case. It correctly translates the effective address to a physical address which it tests against the valid block bitset, skipping invalidation if it is not necessary. As such, the cost of removing the fast path should not be too high.
The Jit64 implementation is retained, though all it does now is emit a call. This is marginally more efficient than simple interpreter fallback, which involves an extra call. The JitArm64 implementation has also been fixed.
The game Happy Feet is fixed by this change, as it loads code in the 7E... address region and depends upon JIT cache invalidation in reponse to dcbf.
https://bugs.dolphin-emu.org/issues/12133
At least on some CPUs (I found out about this from the
Arm Cortex-A76 Software Optimization Guide), using X30
with BLR is one cycle slower than using another register.
Previously, eaddr would only be partially initialized in the ipv6 case.
Even if there's no support for it, we may as well ensure that the
variable always has deterministic initialization.
While we're at it, we can make the parameter a const reference, given no
members are modified.
EmulationActivity has an instance of Settings. If you go to
SettingsActivity from EmulationActivity and change some settings,
the changes get saved to disk, but EmulationActivity's Settings
instance still contains the old settings in its map of all
settings (assuming the EmulationActivity was not killed by the
system to save memory). Then, once you're done playing your
game and exit EmulationActivity, EmulationActivity calls
Settings.saveSettings. This call to saveSettings first overwrites
the entire INI file with its map of all settings (which is
outdated) in order to save any legacy settings that have changed
(which they haven't, since the GUI doesn't let you change legacy
settings while a game is running). Then, it asks the new config
system to write the most up-to-date values available for non-legacy
settings, which should make all the settings be up-to-date again.
The problem here is that the new config system would skip writing
to disk if no settings changes had been made since the last time
we asked it to write to disk (i.e. since SettingsActivity exited).
NB: Calling Settings.loadSettings in EmulationActivity.onResume
is not a working solution. I assume this is because
SettingsActivity saves its settings in onStop and not onPause.
For certain occurrences of nandx/norx, we declare a ReadWrite constraint
on the destination register, even though the value of the destination
register is irrelevant. This false dependency would force the RegCache
to generate a redundant MOV when the destination register wasn't already
assigned to a host register.
Example 1:
BF 00 00 00 00 mov edi,0
8B FE mov edi,esi
F7 D7 not edi
Example 2:
8B 7D 80 mov edi,dword ptr [rbp-80h]
8B FE mov edi,esi
F7 D7 not edi
Also avoid files without a name before the extension (name: ".ini")
from being added to the list because then they wouldn't be saveable
and it would appear with an empty name anyway.
This function has been marked as obsolete. In Qt 6.0 it's removed
entirely, so we must use getContentsMargin() explicitly instead
(margin() would do this for us).
Ditto for setMargin(), in which case we use setContentsMargin instead.
setMargin() would just pass its argument to all four parameters of
setContentsMargin(), so we can do the same.
This literal was deprecated in 5.14.0. Not to mention it wasn't
documented as part of the API either: see the 5.14.0 changelog here:
https://code.qt.io/cgit/qt/qtbase.git/tree/dist/changes-5.14.0?h=v5.14.0
On Qt 6.0 this define is removed entirely. To stay forward compatible,
we can make use of QStringLiteral instead.
This skips a potentially costly loop if volume is 100% or 0%,
as for former there is no need for volume adjustment,
while latter can be solved by specifying a AUDCLNT_BUFFERFLAGS_SILENT flag
This fixes numerous resource leaks, as not every return path cleaned every created resource
Now they are all managed automatically and "commited" to WASAPIStream class fields only
after it's certain they initialized properly
When a game is selected, the option to add a shortcut of the game to the desktop is given. Uses native Windows API since Qt lacks support for adding shortcuts.
FinalizeCarryOverflow didn't maintain XER[OV/SO] properly due to an
oversight. Here's the code it would generate:
0: 9c pushf
1: 80 65 3b fe and BYTE PTR [rbp+0x3b],0xfe
5: 71 04 jno b <jno>
7: c6 45 3b 03 mov BYTE PTR [rbp+0x3b],0x3
000000000000000b <jno>:
b: 9d popf
At first glance it seems reasonable. The host flags are carefully
preserved with PUSHF. The AND instruction clears XER[OV]. Next, an
conditional branch checks the host's overflow flag and, if needed, skips
over a MOV that sets XER[OV/SO]. Finally, host flags are restored with
POPF.
However, the AND instruction also clears the host's overflow flag. As a
result, the branch that follows it is always taken and the MOV is always
skipped. The end result is that XER[OV] is always cleared while XER[SO]
is left unchanged.
Putting POPF immediately after the AND would fix this, but we already
have GenerateOverflow doing it correctly (and without the PUSHF/POPF
shenanigans too). So let's just use that instead.
Happens in Super Mario Sunshine. You could probably do something similar
for b == -1 (like we do for subfic), but I couldn't find any titles that
do this.
- Case 1: d == a
Before:
41 8B C7 mov eax,r15d
41 BF 00 00 00 00 mov r15d,0
44 2B F8 sub r15d,eax
After:
41 F7 DF neg r15d
- Case 2: d != a
Before:
BF 00 00 00 00 mov edi,0
41 2B FD sub edi,r13d
After:
41 8B FD mov edi,r13d
F7 DF neg edi
Consider the case where d and a refer to the same PowerPC register,
which is known to hold an immediate value by the RegCache. We place a
ReadWrite constraint on this register and bind it to an x86 register.
The RegCache then allocates a new register, initializes it with the
immediate, and returns a RCX64Reg for both d and a.
At this point information about the immediate value becomes unreachable.
In the case of subfx, this generates suboptimal code:
Before 1:
BF 1E 00 00 00 mov edi,1Eh <- done by RegCache
8B C7 mov eax,edi
8B FE mov edi,esi
2B F8 sub edi,eax
Before 2:
BE 00 AC 3F 80 mov esi,803FAC00h <- done by RegCache
8B C6 mov eax,esi
8B 75 EC mov esi,dword ptr [rbp-14h]
2B F0 sub esi,eax
The solution is to explicitly handle the constant a case before having
the RegCache allocate registers for us.
After 1:
8D 7E E2 lea edi,[rsi-1Eh]
After 2:
8B 75 EC mov esi,dword ptr [rbp-14h]
81 EE 00 AC 3F 80 sub esi,803FAC00h
The special case doesn't appear to make a significant difference in any games, and the current implementation has a (minor, fixable) issue that breaks Super Mario Sunshine (both with a failed assertion (https://bugs.dolphin-emu.org/issues/11742) and a rendering issue (https://bugs.dolphin-emu.org/issues/7476)). Hardware testing wasn't able to reproduce the special case, either, so it may just not exist.
PR #9315 contains a fixed implementation of the special case on all video backends, and can serve as a basis for it being reintroduced if it is found to exist under more specific circumstances. For now, I don't see a reason to keep it present.
-If adding 2 devices with the same name, they their unique id wouldn't be increased, causing a conflict.
-Removing a device wouldn't actually remove it from the internal devices list because the list of devices had already been updated when going through it.
-It was possible to remove devices belonging to other sources by adding a device with the same name and then removing it.
The name was confusing as changing it at runtime would not change the window to fullscreen, as it effectively only affects the start of the emulation.
Also blocked the ability to change it when the emulation is running, to be more inline with other similar settings, like "Render to main Window".
Also provides operator!= for logical symmetry.
We can also take the arguments by value, as the arguments are trivially
copyable enum values which fit nicely into registers already.
https://bugs.dolphin-emu.org/issues/6749
This change fixes the scratchy audio in Teenage Mutant Ninja Turtles (SX7E52/SX7P52). The game starts an audio interface DMA with an unaligned address, and because Dolphin was not masking off the low 5 bits of AUDIO_DMA_START_LO, all future AI DMAs were misaligned. To understand why, it is instructive to refer to AUDIO_InitDMA() in libogc, which behaves the same as the official SDK:
_dspReg[25] = (_dspReg[25]&~0xffe0)|(startaddr&0xffff);
The implementation does not mask off the low bits of the passed in value before it ORs them with low bits of the current register value. Therefore, if they are not masked off by the hardware itself, they become permanently stuck once set.
Adding a write mask for AUDIO_DMA_START_LO is enough to fix the bug in TMNT, but I decided to run some tests on GC and Wii to find the correct write masks for the surrounding registers, as only a couple were already being masked. Dolphin has gotten away with not masking the rest because many are already A) masked on read (or never read) by the SDK and/or B) masked on use (or never used) in Dolphin.
This leaves just three registers where the difference may be observable: AR_DMA_CNT_H and AUDIO_DMA_START_HI/LO.
operator[] performs a default construction if an object at the given key
doesn't exist before overwriting it with the one we provide in operator=
insert_or_assign performs optimal insertion by avoiding the default
construction if an entry doesn't exist.
Not a game changer, but it is essentially a "free" change.
Allows lookups to be done with std::string_view or any other string
type. This allows for non-allocating strings to be used with the name
lookup without needing to construct a std::string.
Cleans up some locks that explicitly specify the recursive mutex type in
it. Meant to be included with the previous commit that cleaned out
regular mutexes, but I forgot.
This code was storing references to patch entries which could move around in memory if a patch was erased from the middle of a vector or if the vector itself was reallocated. Instead, NewPatchDialog maintains a separate copy of the patch entries which are committed back to the patch if the user accepts the changes.
These games are erroneously zeroing buffers before they can be fully copied to ARAM by DMA. The responsible memset() calls are followed by a call to DVDRead() which issues dcbi instructions that effectively cancel the memset() on real hardware. Because Dolphin lacks dcache emulation, the effects of the memset() calls are observed, which causes missing audio.
In a comment on the original bug, phire noted that the issue can be corrected by simply nop'ing out the offending memset() calls. Because the games dynamically load different .rel executables based on the character and/or language, the addresses of these calls can vary.
To deal generally with the problem of code being dynamically loaded to fixed, known addresses, the patch engine is extended to support conditional patches which require a match against a known value. This sort of thing is already achievable with Action Replay/Gecko codes, but their use depends on enabling cheats globally in Dolphin, which is not a prerequisite shared by patches.
Patches are included for every region, character, and language combination. They are enabled by default.
The end result is an approximation of the games' behavior on real hardware without the associated complexity of proper dcache emulation.
https://bugs.dolphin-emu.org/issues/9840
The config version should always be incremented whenever config is
changed, regardless of callbacks being suppressed or not.
Otherwise, getters can return stale data until another config change
(with callbacks enabled) happens.
C++17 allows omitting the mutex type, which makes for both less reading
and more flexibility (e.g. The mutex type can change and all occurrences
don't need to be updated).
Allows the analyzer to exist independently of the DSP structure. This
allows for unit-tests to be created in a nicer manner.
SDSP is only necessary during the analysis phase, so we only need to
keep a reference around to it then as opposed to the entire lifecycle of
the analyzer.
This also allows the copy/move assignment operators to be defaulted, as
a reference member variable prevents that.
Now that we have the convenience functions around the flag
bit manipulations, there's no external usages of the flags, so we can
make these private to the analyzer implementation.
Now the Analyzer namespace is largely unnecessary and can be merged with
the DSP namespace in the next commit.
This commit implements the following commands:
* open
* close
* GetMode
* SetLinkState (used to actually trigger scanning)
* GetLinkState (used to check if the driver is in the expected state)
* GetInfo
* RecvFrame and RecvNotification (stubbed)
* Disassociate (stubbed)
GetInfo was already implemented, but the structure wasn't initialized
correctly so the info was being rejected by official titles.
That has also been fixed in this commit.
Some of the checks may seem unimportant but official titles actually
require WD to return error codes... Failing to do so can cause hangs
and softlocks when DS communications are shut down.
This minimal implementation is enough to satisfy the Mii channel
and all other DS games, except Tales of Graces (https://dolp.in/i11977)
which still softlocks because it probably requires us to actually
feed it frame data.
NCD returns an error if it receives a request to lock the driver
when it is already locked.
Emulating this may seem pointless, but it turns out PPC-side code
expects NCD to return an error and will immediately fail and stop
initialising wireless stuff if NCD succeeds.
Localizes code that modifies m_dsp into the struct itself. This reduces
the overal coupling between DSPCore and SDSP by reducing access to its
member variables.
This commit is only code movement and has no functional changes.
An unfortunately large single commit that deglobalizes the DSP code.
(which I'm very sorry about).
This would have otherwise been extremely difficult to separate due to
extensive use of the globals in very coupling ways that would result in
more scaffolding to work around than is worth it.
Aside from the video code, I believe only the DSP code is the hairiest
to deal with in terms of globals, so I guess it's best to get this dealt
with right off the bat.
A summary of what this commit does:
- Turns the DSPInterpreter into its own class
This is the most involved portion of this change.
The bulk of the changes are turning non-member functions into member
functions that would be situated into the Interpreter class.
- Eliminates all usages to globals within DSPCore.
This generally involves turning a lot of non-member functions into
member functions that are either situated within SDSP or DSPCore.
- Discards DSPDebugInterface (it wasn't hooked up to anything,
and for the sake of eliminating global state, I'd rather get rid of
it than think up ways for this class to be integrated with
everything else.
- Readjusts the DSP JIT to handle calling out to member functions.
In most cases, this just means wrapping respective member function
calles into thunk functions.
Surprisingly, this doesn't even make use of the introduced System class.
It was possible all along to do this without it. We can house everything
within the DSPLLE class, which is quite nice =)
Shifting zero by any amount always gives zero.
Before:
41 B9 00 00 00 00 mov r9d,0
41 8B CF mov ecx,r15d
49 C1 E1 20 shl r9,20h
49 D3 F9 sar r9,cl
49 C1 E9 20 shr r9,20h
After:
Nothing, register is set to constant zero.
Before:
41 B8 00 00 00 00 mov r8d,0
41 8B CF mov ecx,r15d
49 C1 E0 20 shl r8,20h
49 D3 F8 sar r8,cl
41 8B C0 mov eax,r8d
49 C1 E8 20 shr r8,20h
44 85 C0 test eax,r8d
0F 95 45 58 setne byte ptr [rbp+58h]
After:
C6 45 58 00 mov byte ptr [rbp+58h],0
Occurs a bunch of times in Super Mario Sunshine. Since this is an
arithmetic shift a similar optimization can be done for constant -1
(0xFFFFFFFF), but I couldn't find any game where this happens.
Shifting zero by any amount always gives zero.
Before:
41 BF 00 00 00 00 mov r15d,0
8B CF mov ecx,edi
49 D3 E7 shl r15,cl
45 8B FF mov r15d,r15d
After:
Nothing, register is set to constant zero.
All games I've tried hit this optimization on launch. In Soul Calibur II
it occurs very frequently during gameplay.
Much like we did for srawx. This was already implemented on JitArm64.
Before:
B8 00 00 00 00 mov eax,0
8B F0 mov esi,eax
C1 E8 1F shr eax,1Fh
23 C6 and eax,esi
D1 FE sar esi,1
88 45 58 mov byte ptr [rbp+58h],al
After:
C6 45 58 00 mov byte ptr [rbp+58h],0
If both input registers hold known values at compile time, we can just
calculate the result on the spot.
Code has mostly been copied from JitArm64 where it had already been implemented.
Before:
BF FF FF FF FF mov edi,0FFFFFFFFh
8B C7 mov eax,edi
C1 FF 10 sar edi,10h
C1 E0 10 shl eax,10h
85 F8 test eax,edi
0F 95 45 58 setne byte ptr [rbp+58h]
After:
C6 45 58 01 mov byte ptr [rbp+58h],1
More efficient code can be generated if the shift amount is known at
compile time. We can once again take advantage of shifts with the shift
amount in an 8-bit immediate to eliminate ECX as a scratch register,
reducing register pressure and removing the occasional spill. We can
also do 32-bit shifts instead of 64-bit operations.
We recognize four distinct cases:
- The special case where we're dealing with the PowerPC's quirky shift
amount masking. If the shift amount is a number from 32 to 63, all
bits are shifted out and the result it either all zeroes or all ones.
Before:
B9 F0 FF FF FF mov ecx,0FFFFFFF0h
8B F7 mov esi,edi
48 C1 E6 20 shl rsi,20h
48 D3 FE sar rsi,cl
8B C6 mov eax,esi
48 C1 EE 20 shr rsi,20h
85 F0 test eax,esi
0F 95 45 58 setne byte ptr [rbp+58h]
After:
8B F7 mov esi,edi
C1 FE 1F sar esi,1Fh
0F 95 45 58 setne byte ptr [rbp+58h]
- The shift amount is zero. Not calculation needs to be done, just clear
the carry flag.
Before:
B9 00 00 00 00 mov ecx,0
49 C1 E5 20 shl r13,20h
49 D3 FD sar r13,cl
41 8B C5 mov eax,r13d
49 C1 ED 20 shr r13,20h
44 85 E8 test eax,r13d
0F 95 45 58 setne byte ptr [rbp+58h]
After:
C6 45 58 00 mov byte ptr [rbp+58h],0
- The carry flag doesn't need to be computed. Just do the arithmetic
shift.
Before:
B9 02 00 00 00 mov ecx,2
48 C1 E7 20 shl rdi,20h
48 D3 FF sar rdi,cl
48 C1 EF 20 shr rdi,20h
After:
C1 FF 02 sar edi,2
- The carry flag must be computed. In addition to the arithmetic shift,
we do a shift to the left and and them together to know if any ones
were shifted out. It's still better than before, because we can do
32-bit shifts.
Before:
B9 02 00 00 00 mov ecx,2
49 C1 E5 20 shl r13,20h
49 D3 FD sar r13,cl
41 8B C5 mov eax,r13d
49 C1 ED 20 shr r13,20h
44 85 E8 test eax,r13d
0F 95 45 58 setne byte ptr [rbp+58h]
After:
41 8B C5 mov eax,r13d
41 C1 FD 02 sar r13d,2
C1 E0 1E shl eax,1Eh
44 85 E8 test eax,r13d
0F 95 45 58 setne byte ptr [rbp+58h]
More efficient code can be generated if the shift amount is known at
compile time. Similar optimizations were present in JitArm64 already,
but were missing in Jit64.
- By using an 8-bit immediate we can eliminate the need for ECX as a
scratch register, thereby reducing register pressure and occasionally
eliminating a spill.
Before:
B9 18 00 00 00 mov ecx,18h
41 8B F7 mov esi,r15d
48 D3 E6 shl rsi,cl
8B F6 mov esi,esi
After:
41 8B CF mov ecx,r15d
C1 E1 18 shl ecx,18h
- PowerPC has strange shift amount masking behavior which is emulated
using 64-bit shifts, even though we only care about a 32-bit result.
If the shift amount is known, we can handle this special case
separately, and use 32-bit shift instructions otherwise. We also no
longer need to clear the upper 32 bits of the register.
Before:
BE F8 FF FF FF mov esi,0FFFFFFF8h
8B CE mov ecx,esi
41 8B F4 mov esi,r12d
48 D3 E6 shl rsi,cl
8B F6 mov esi,esi
After:
Nothing, register is set to constant zero.
- A shift by zero becomes a simple MOV.
Before:
BE 00 00 00 00 mov esi,0
8B CE mov ecx,esi
41 8B F3 mov esi,r11d
48 D3 E6 shl rsi,cl
8B F6 mov esi,esi
After:
41 8B FB mov edi,r11d
More efficient code can be generated if the shift amount is known at
compile time. Similar optimizations were present in JitArm64 already,
but were missing in Jit64.
- By using an 8-bit immediate we can eliminate the need for ECX as a
scratch register, thereby reducing register pressure and occasionally
eliminating a spill.
Before:
B9 18 00 00 00 mov ecx,18h
45 8B C1 mov r8d,r9d
49 D3 E8 shr r8,cl
After:
45 8B C1 mov r8d,r9d
41 C1 E8 18 shr r8d,18h
- PowerPC has strange shift amount masking behavior which is emulated
using 64-bit shifts, even though we only care about a 32-bit result.
If the shift amount is known, we can handle this special case
separately, and use 32-bit shift instructions otherwise.
Before:
B9 F8 FF FF FF mov ecx,0FFFFFFF8h
45 8B C1 mov r8d,r9d
49 D3 E8 shr r8,cl
After:
Nothing, register is set to constant zero.
- A shift by zero becomes a simple MOV.
Before:
B9 00 00 00 00 mov ecx,0
45 8B C1 mov r8d,r9d
49 D3 E8 shr r8,cl
After:
45 8B C1 mov r8d,r9d
The enumerated LOG_TYPE "OSREPORT" is currently used in both EXI_DeviceIPL.cpp and HLE_OS.cpp. In many games, the multitude of game functions detected by HLE_OS.cpp for OSREPORT logging results in poor log readability. This Pull Request remedies that by adding a new enumerated LOG_TYPE "OSREPORT_HLE" for log usage in HLE_OS.cpp.
In the future, further changing how logging in HLE_OS.cpp works may be desirable. As it is, game functions are detected that send a single character to the log. This is a major source of poor readability.
Introduces the system class that will eventually contain all relevant
system state, as opposed to everything being distributed all over the
place as global variables.
Throughout the codebase we have code that from its interface-view, does
not actually require its dependencies to be described in the interface,
and we routinely run into issues with initialization where we sometimes
make use of a facility before it's been initialized, which leads to
annoying to debug cases, because the reader needs to run through the
codebase and see what order things get initialized in, and how they're
being used. This is particularly a frequent issue in the video code.
Further, we also have a lot of code that makes use of file-scope
variables (many of which are non-trivial), which must all be default
initialized before the application can actually enter main(). While this
may not be a huge issue in itself, some of these are allocating, which
means that the application may need to use memory that it otherwise
wouldn't need to (e.g. when a game isn't running, this excess memory is
being used).
Being able to wrap all these subsystems into objects would be nicer,
since they can be constructed when they're actually needed. Them being
objects also means we can better express dependencies on subsystems as
types directly in the interface, making them explicit to the reader
instead of a change randomly blowing up, said reader inspecting it, and finding
out that something needed to be initialized beforehand. With the global
turned into a function parameter, the dependency is explicit and they
know just by reading it, that the given subsystem needs to be in a valid
state before calling the function.
For a prior example of an emulator that has moved to this model, see
yuzu, which has been migrated off of global variables all over the place
and replaced with a system instance (which has now reached the stage,
where the singleton can be removed).
We want to clear/memset the padding bytes, not just each member,
so using assignment or {} initialization is not an option.
To silence the warnings, cast the object pointer to u8* (which is not
undefined behavior) to make it explicit to the compiler that we want
to fill the object representation.
TunTap has recently become unmaintained, and it seems Apple wants developers to move away from kexts in general. TunTap currently takes some finagling to work on Catalina, and it may not work at all on Big Sur, necessitating a non-kext-based solution. Fortunately, fake Ethernet devices were introduced in Sierra and can be used similarly to tap adapters. This commit adds a new type of BBA interface implementation which uses fake Ethernet devices via tapserver (https://github.com/fuzziqersoftware/tapserver) to communicate with the host. This implementation was tested with PSO Episodes I & II, which can successfully connect to a private server running locally.
This implementation is only available on macOS, since that's the only place it's needed - Windows/Linux/Unix are unaffected by TunTap being deprecated.
Make sure m_is_populating_devices is true when a WM_INPUT_DEVICE_CHANGE
event is received directly on the ciface thread, so that callbacks do
not occur while removing devices. This breaks a hold-and-wait deadlock
between the ciface thread and the CPU thread when using emulated
Wiimotes.
Co-authored-by: brainleq <brainleq@users.noreply.github.com>
Co-authored-by: oldmud0 <oldmud0@users.noreply.github.com>
If we want to enable codes in the default game INIs,
we should have some way for users to disable them.
This commit accomplishes that by adding a *_Disabled
section corresponding to each *_Enabled section.
In order to reach the middle guard (at m_stack_base + GUARD_OFFSET)
before the bottom guard (at m_stack_base), the stack pointer
must start at an address which is higher than the middle guard.
It also didn't make sense that we were allocating memory
and then not using the top part of it.
Nintendo's official title installation code and ES both only look at
content IDs but we should probably check for content hashes in addition
to checking for IDs for at least two reasons:
1. Some of the installed contents could be corrupted -- this cannot be
easily detected without checking hashes.
2. Some mod distributors do not bother to update content IDs, which
means that installing updates from the UI would not actually
update the installed game. This is confusing for users.
To keep the existing semantic (for IOS especially), the new content
hash checks are opt-in for callers of GetStoredContentsFromTMD.
This commit changes WiiUtils's WAD installation logic to enable
the content hash checks.
Adds a flag to File::Delete and File::DeleteDir functions to control
whether a console warning is emitted when the file or directory doesn't
exist. The flag is optional and true by default to match current behavior.
Makes File::DeleteDir return true when attempting to delete a
nonexistent path.
The purpose of DeleteDir is to ensure the path doesn't exist after the
call, which is better reflected by the new return value. Additionally,
none of the current callers actually check the return value so this
won't break any existing code.
Fixes https://bugs.dolphin-emu.org/issues/12327.
When we started using fmt in CheckExternalExceptions, JitArm64
mysteriously stopped working even though the code path where
fmt was used never was reached. This is because the compiler
added a function prologue and epilogue to set up the stack,
since the code path that used fmt required the use of the stack.
However, the breakage didn't actually have anything to do
with the usage of the stack in itself, but rather with the
compiler's insertion of a stack canary. In the function
epilogue, a cmp instruction was inserted to check that the
stack canary had not been overwritten during the execution
of the function. This cmp instruction overwriting the status
flags ended up having a disastrous side effect once execution
returned to code emitted by JitArm64::WriteExceptionExit.
JitArm64's dispatcher contains a branch to the "do_timing"
code which is intended to be taken if the PPC downcount is
negative. However, the dispatcher doesn't update the status
flags on its own before this conditional branch, but rather
expects the calling code to have set them as a side effect
of DoDownCount. The root cause of our bug was that
JitArm64::WriteExceptionExit was calling DoDownCount before
Check(External)Exceptions instead of after.
1. Comparing string_views does not behave the same as strncmp
in the case where SConfig's game ID is longer than 6 chars.
2. DTMHeader::GetGameID wasn't excluding null bytes for game IDs
shorter than 6 chars.
3. == was accidentally used instead of !=.
This issue is both severe and surprisingly difficult to find
the root cause of, so I think it would make sense to add a simple
hotfix for now. https://bugs.dolphin-emu.org/issues/12327
Fallback Region
A user-selected fallback to use instead of the default PAL
This is used for unknown region or region free titles to give them
the ability to force region to use. This replaces the current fallback region
of PAL. This can be useful if a user is trying to play a region free
tilte that is originally NTSC and expects to be run at NTSC speeds. This
may be done when a user attempts to dump a WAD of their own without
understanding the settings they have chosen, or could be an intentional
decision by a developer of a ROM hack that can be injected into a
Virtual Console WAD.
Remove using System Menu region being checked in GetFallbackRegion
Use DiscIO::Region instead of std::String for fallback
Add explanation text for Fallback Region
Unfortunately, adding a DEBUG_ASSERT_MSG_FMT isn't actually possible
right now because of compiler bugs:
https://github.com/dolphin-emu/dolphin/pull/9284
We could require a newer version of GCC (10) but that would require
updating GCC on the build machines.
For what it's worth, older versions of GCC (8, 9) are broken in
many ways: adding constexpr to some Matrix functions causes GCC 8
to generate bugged code that causes the Wii IR pointer to disappear,
which means that the generated builds are already unusable
(see https://dolp.in/i12324).
Additionally, we've already had to add workarounds for those versions
in the format macros to fix compilation bugs. This time, it looks like
workarounds won't cut it; even applying the workaround
described in https://github.com/fmtlib/fmt/pull/1580 does not help.
On Windows, when the Rename function fails to replace an existing file
it will now retry the operation multiple times with increasingly long
delays between attempts. This fixes transient rename failures.
I've been getting sporadic yet annoyingly frequent errors saying:
'IOS_FS: Failed to rename temporary FST file'
These typically appear on startup but I've also gotten them randomly.
Investigation shows this happens when the Windows ReplaceFile function
returns the error ERROR_UNABLE_TO_REMOVE_REPLACED. That happens in the
context of using ReplaceFile to perform an atomic file overwrite, which
is required when saving updates to a file to avoid corruption. The
error mainly happens with the /Wii/fst.bin file but I've seen it
happen with multiple other files as well.
I haven't been able to definitively pin down why the error occurs,
though online discussions suggest antivirus scanning may be a major
culprit. That said, I've excluded the Dolphin folder from Windows
Defender scans to no avail and don't have any other antivirus running,
so this is likely to be a problem others are experiencing as well.
The number and duration of retry delays is arbitrary but I feel like a
combined second or so in the worst case is an acceptable tradeoff for
the reduction (actually elimination in my experience) of those errors.
This is even more true when you consider the time it takes to read and
dismiss the error dialogs.
Converts the remaining PowerPC code over to fmt-capable logging.
Now, all that's left to convert over are the lingering remnants within
the frontend code.
The way Config::Get works in master, it first calls
Config::GetActiveLayerForConfig which searches for the
setting in all layers, and then calls Config::Layer::Get
which searches for the same setting again within the given
layer. We can remove this second search by combining the
logic of Config::GetActiveLayerForConfig and
Config::Layer::Get into one function.
gameID isn't null terminated since it is just an std::array<char, 6>
and .data() returns a char* so {fmt} would go way beyond the bounds of
the array when it attempts to determine the length of the string.
The fix is to pass a std::string_view to {fmt}. This commit adds
a GetGameID() function that can also be used to simplify
string comparisons.
Cel-damage depends on lighting being calculated for the first channel
even though there is no color in the vertex format (defaults to the
material color). If lighting for the channel is not enabled, the vertex
will use the default color as before.
The default value of the color is determined by the number of elements in
the vertex format. This fixes the grey cubes in Super Mario Sunshine.
If the color channel count is zero, we set the color to black before the
end of the vertex shader. It's possible that this would be undefined
behavior on hardware if a vertex color index that was greater than the
channel count was used within TEV.
PR #9260 made the MsgAlert macros use lambdas so that local
constexpr variables can be added while keeping the ability to
return a boolean from the macros.
Unfortunately, C++17 forbids referring to structured bindings in lambda
captures. This is fixed in P1091R3 but we cannot rely on C++20 yet...
In CreateDescriptorSetLayouts(), one less dynamic binding is created when
bSupportsGeometryShaders=false. Reduce the dynamicOffsetCount argument by
one in that case. Avoids this validation error:
Attempting to bind 3 descriptorSets with 2 dynamic descriptors, but
dynamicOffsetCount is 3. It should exactly match the number of dynamic
descriptors. The Vulkan spec states: dynamicOffsetCount must be equal to
the total number of dynamic descriptors in pDescriptorSets
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
[Applied clang-format]
Signed-off-by: Léo Lam <leo@leolam.fr>
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any runtime or compile-time error [1].
As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.
[1] https://github.com/fmtlib/fmt/issues/492
We want to use positional arguments in translatable strings
that have more than one argument so that translators can change
the order of them, but the question is: Should we also use
positional arguments in translatable strings with only one
argument? I think it makes most sense that way, partially
so that translators don't even have to be aware of the
non-positional syntax and partially because "translatable
strings use positional arguments" is an easier rule for us
to remember than "transitional strings which have more than
one argument use positional arguments". But let me know if
you have a different opinion.
tr calls with more than one argument would have a 0x04 byte
in the returned string when no translation was found
(which always is the case when using Dolphin in English).
They are unused, since there is no C++ code that touches
these settings. See the discussion in PR 9152, which is
a PR that adds a lot more Android-specific settings.
I moved it from the main settings screen to the in-game menu
in PR 8439 so that it could be changed while a game is running,
but now that the main settings can be accessed while a game is
running, there's no reason to not put it in the main settings.
https://bugs.dolphin-emu.org/issues/12067
Adds an interface that uses fmt under the hood, which is much more
flexible than printf, particularly for localization purposes, given fmt
supports positional formatters in a cross-platform manner out of the box
with no configuration necessary.
Time for yet another new iteration of working around the
"surface destruction during boot" problem...
This time, the strategy is to use a mutex in MainAndroid.cpp.
Now that we've converted all of the shader generators over to using fmt,
we can drop the old Write() member function and perform a rename
operation on the WriteFmt() to turn it into the new Write() function.
All changes within this are the removal of a <cstdarg> header, since the
previous printf-based Write() required it, and renaming. No functional
changes are made at all.
Noticed missing include as a build failure on gcc-11:
```
[ 15%] Building CXX object Source/Core/Common/CMakeFiles/common.dir/Config/Config.cpp.o
Source/Core/Common/Config/Config.cpp:23:24:
error: 'unique_lock' in namespace 'std' does not name a template type
23 | using WriteLock = std::unique_lock<std::shared_mutex>;
| ^~~~~~~~~~~
Source/Core/Common/Config/Config.cpp:11:1:
note: 'std::unique_lock' is defined in header '<mutex>';
did you forget to '#include <mutex>'?
```
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Completes the migration over to using the fmt-formatting WriteFmt
function. The next PR will rename all usages of WriteFmt, while
simultaneously getting rid of the old printf code.
Unfortunately, compilers will issue warnings when using offsetof with
non-standard layout types even when offsetof actually works fine here;
just having a virtual function is enough to trigger the warning...
Let's just stop the scan threads explicitly in destructors instead of
relying on member destruction order.
Much of these classes are operating on integral types and are pretty
standard behavior as far as vectors go. Some member functions can be
made constexpr to make them more flexible and allow them to be used in
constexpr contexts.
Replace it with a function-local static that is initialized on first
use. This gets rid of a global variable and removes the need for
manual initialization in UICommon.
This commit also replaces the weird find_if that looks for a non-null
unique_ptr with a simple "is vector empty" check considering that
none of the pointers can be null by construction.
As in the comment, the panning was denaturalizing the volume (when the panning was at 0).
Unless users actually want to use panning, their wii mote volume should not be reduced, it should come out at the same volume the samples were.
If users want to reduce the volume of the wii mote speakers, they can do so from the wii home menu.
I opted for this instead of adding another setting "Enable Panning" as it would have been confusing, and the changes in the panning formula are unlikely to have any negative effect, as it still works.
Provides a basic extension to the interface to begin migration off of
the printf-based logging system.
Everything will go through macros with the same style naming as the old
logging system, except the macros will have the _FMT suffix, while the
migration is in process.
This allows for peacemeal migration over time instead of pulling
everything out and replacing it all in a single pull request, which
makes for much easier reviewing.
Common shouldn't be depending on APIs in Core (in this, case depending
on the PowerPC namespace). Because of the poor separation here, this
moves OSThread functionality into core, so that it resolves the implicit
dependency on core.
Continues migration of the shader generators over to fmt.
With this, all that's left to move over are the pixel shaders (regular
and ubershader variants)