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)
Fixes the following warning:
../../../../../../Core\DiscIO/DirectoryBlob.h:156:3: warning: explicitly defaulted move constructor is implicitly deleted [-Wdefaulted-function-deleted]
DirectoryBlobReader(DirectoryBlobReader&&) = default;
^
../../../../../../Core\DiscIO/DirectoryBlob.h:205:22: note: move constructor of 'DirectoryBlobReader' is implicitly deleted because field 'm_encryption_cache' has a deleted move constructor
WiiEncryptionCache m_encryption_cache;
^
In case someone wants to be very careful with how much bandwidth
they use or with what data GameTDB.com collects on you.
This is already an option in DolphinQt (though in DolphinQt it
will switch entirely from using covers to banners when turned off).
Once nice benefit of fmt is that we can use positional arguments
in localizable strings. This a feature which has been
requested for the Korean translation of strings like
"Errors were found in %zu blocks in the %s partition."
and which will no doubt be useful for other languages too.
Noticed missing include as a build failure on gcc-11:
```
[ 26%] Building CXX object Source/Core/DiscIO/CMakeFiles/discio.dir/WIACompression.cpp.o
../../../../Source/Core/DiscIO/WIACompression.cpp: In lambda function:
../../../../Source/Core/DiscIO/WIACompression.cpp:170:31: error: 'numeric_limits' is not a member of 'std'
170 | std::min<size_t>(std::numeric_limits<unsigned int>().max(), x));
| ^~~~~~~~~~~~~~
../../../../Source/Core/DiscIO/WIACompression.cpp:170:46: error: expected primary-expression before 'unsigned'
170 | std::min<size_t>(std::numeric_limits<unsigned int>().max(), x));
| ^~~~~~~~
```
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
By calling ZSTD_CCtx_setPledgedSrcSize, we can let zstd know
how large a chunk is going to be before which start compressing
it, which lets zstd avoid allocating more memory than needed
for various internal buffers. This greatly reduces the RAM usage
when using a high compression level with a small chunk size,
and doesn't have much of an effect in other circumstances.
A side effect of calling ZSTD_CCtx_setPledgedSrcSize is that
zstd by default will write the uncompressed size into the
compressed data stream as metadata. In order to save space,
and since the decompressed size can be figured out through
the structure of the RVZ format anyway, we disable writing
the uncompressed size by setting ZSTD_c_contentSizeFlag to 0.
Unlike Super Paper Mario, this game doesn't crash as soon as you
try to start it, but rather if you try to skip a certain cutscene.
Thanks to JMC for letting me know about this.
For the non-packed variant of this instruction, a MOVSD instruction was
generated to copy only the lower 64 bits of XMM1 to the destination
register. This was done in order to keep the destination register's
upper half intact.
However, when register c and the destination register are the same,
there is no need for this copy. Because the registers match and due to
the way the mask is generated, BLENDVPD will end up taking the upper
half from the destination register, as intended.
Additionally, the MOVAPS to copy Rc into XMM1 can also be skipped.
Before:
66 0F 57 C0 xorpd xmm0,xmm0
F2 41 0F C2 C6 06 cmpnlesd xmm0,xmm14
41 0F 28 CE movaps xmm1,xmm14
66 41 0F 38 15 CA blendvpd xmm1,xmm10,xmm0
F2 44 0F 10 F1 movsd xmm14,xmm1
After:
66 0F 57 C0 xorpd xmm0,xmm0
F2 41 0F C2 C6 06 cmpnlesd xmm0,xmm14
66 45 0F 38 15 F2 blendvpd xmm14,xmm10,xmm0
For the non-packed variant of this instruction, a MOVSD instruction was
generated to copy only the lower 64 bits of XMM1 to the destination
register. This was done in order to keep the destination register's
upper half intact.
However, when register c and the destination register are the same,
there is no need for this copy. Because the registers match and due to
the way the mask is generated, VBLENDVPD will end up taking the upper
half from the destination register, as intended.
Before:
66 0F 57 C0 xorpd xmm0,xmm0
F2 41 0F C2 C6 06 cmpnlesd xmm0,xmm14
C4 C3 09 4B CA 00 vblendvpd xmm1,xmm14,xmm10,xmm0
F2 44 0F 10 F1 movsd xmm14,xmm1
After:
66 0F 57 C0 xorpd xmm0,xmm0
F2 41 0F C2 C6 06 cmpnlesd xmm0,xmm14
C4 43 09 4B F2 00 vblendvpd xmm14,xmm14,xmm10,xmm0
CommandLineParse expects UTF-8 strings. (QApplication, on the
other hand, seems to be designed so that you can pass in the
char** argv untouched on Windows and get proper Unicode handling.)
Any call to Config::SetCurrent will cause the relevant setting
to show up as overridden in the Android GUI, which can be confusing,
so let's not do it when the new value is the same as the original.
SYSCONF very much is saveable. Whether it's in IsSettingSaveable
or not hasn't mattered until now since the SYSCONF settings use
separate config loader code that doesn't check IsSettingSaveable,
but the next commit will require SYSCONF to be marked as saveable.
Adding AmdPowerXpressRequestHighPerformance
This will allow AMD drivers to detect the request to use the dGPU instead of the iGPU on compatible hybrid graphics systems.
Reference: https://community.amd.com/thread/169965
Fixes https://bugs.dolphin-emu.org/issues/12245.
I considered making a change to DolphinQt instead of
the core, but then additional effort would've been
required to add the same fix to the Android GUI once
we start using the new config system there.
It's possible (but rare) for a WIA or RVZ file to support
this for some partitions but not all, and for the game and
the blob code to disagree on how large a partition is.
In particular, I wanted to do change this in
AudioInterface::Init so that dumped GC audio doesn't need
to have a file split (changing from 32000 Hz to 32029 Hz)
when the emulated software initializes the AI registers.
I've also made the same change to DI's DTK code.
The heuristic was not allocating enough space for Metroid: Other M,
at least when using the default settings. (This didn't break the
file, it just caused some headers to be placed at the end of the
file instead of at the start and wasted a few hundred kilobytes.)
The new hash check catches essentially all desync problems
that VolumeVerifier can catch, so from the user's perspective,
such problems will result in Dolphin refusing to start the
game on netplay rather than actually getting a desync.
This moves the scan thread logic and variables into a separate
ScanThread class. By turning ScanThread instances into members of the
most derived class, this ensures that the scan thread is always
properly stopped when the most derived class is destructed and fixes
a race condition that could cause the scan thread to call virtual
member functions from a derived class whose members have already
been destructed.
A drawback of this approach is that the scan thread must be the last
member variable, so this commit also adds static assertions to ensure
that the assumption stays valid.
The "FindLibLZMA.cmake" module in CMake versions prior to 3.14 do not
set an alias like how Externals/liblzma/CMakeLists.txt does, so builds
performed using one of those older CMake versions will fail if the
system LZMA library is detected. To fix this, we need to link against
"lzma" instead of "LibLZMA::LibLZMA".
Fixes: b59ef81a7e ("WIA: Implement bzip2, LZMA, and LZMA2 decompression")
When I first made VolumeVerifier, I figured that the distinction
between an unsigned ticket and an unsigned TMD was a technical
detail that users would have no reason to care about. However,
while this might be true for discs, it isn't equally true for
WADs, due to the widespread practice of fakesigning tickets to
set the console ID to 0. This practice does not require
fakesigning the TMD (though apparently people do it anyway,
at least sometimes...), and the presence of a correctly signed
TMD is a useful indicator that the contents have not been
tampered with, even if the ticket isn't correctly signed.
FCVT doesn't necessarily round to zero, so the result
might be inaccurate if we use it. To ensure correct
rounding, we use FCVTS from double FPR to 32-bit GPR.
Unfortunately, FCVTS can't do double FPR to single FPR.
This is now unused. Seems like it was an improper fix
(there would be a race if saving the screenshot took longer
than 2 seconds) back when it was used too.
The intent here is to generate a more compact instruction if a 32-bit
immediate can be zero-extended to the desired 64-bit immediate.
Nowadays the emitter is smart enough to do this for us, so this logic is
redundant.
There's no need to load the 64-bit immediate into a temporary register.
x64 will sign-extend 32-bit immediates to 64 bits, giving us the exact
value we need in this case.
48 C7 C0 00 00 FF FF mov rax,0FFFFFFFFFFFF0000h
48 21 C2 and rdx,rax
48 81 E2 00 00 FF FF and rdx,0FFFFFFFFFFFF0000h
- LEA is a bit silly when the source and the destination are the same. A
simple ADD or SHL will do in those cases.
66 8D 04 45 00 00 00 00 lea ax,[rax*2]
66 03 C0 add ax,ax
48 8D 04 00 lea rax,[rax+rax]
48 03 C0 add rax,rax
66 8D 14 D5 00 00 00 00 lea dx,[rdx*8]
66 C1 E2 03 shl dx,3
- When scaling by 2, consider summing the register with itself instead.
The former always needs a 32-bit displacement, so the sum is more
compact.
66 8D 14 45 00 00 00 00 lea dx,[rax*2]
66 8D 14 00 lea dx,[rax+rax]
Other than the controller settings and JIT debug settings,
these are the only settings which were defined in Java code
but not defined in the new config system in C++. (There are
still a lot of settings that are defined in the new config
system but not yet saveable in the new config system, though.)
Instead of comparing the game ID, revision, disc number and name,
we can compare a hash of important parts of the disc including
all the aforementioned data but also additional data such as the
FST. The primary reason why I'm making this change is to let us
catch more desyncs before they happen, but this should also fix
https://bugs.dolphin-emu.org/issues/12115. As a bonus, the UI can
now distinguish the case where a client doesn't have the game at
all from the case where a client has the wrong version of the game.
Turns out, Gamecube games actually do check DILENGTH, and if DILENGTH is at 0, they'll think the transfer completed successfully even if DEINT is used, since after all, surely that means everything was sent. That caused all sorts of issues, from audio looping when a disc is removed since it's re-using the same buffer to just flat-out crashing instead of showing the disc removed screen.
In particular:
- Trying to play audio in a non-ready state returns the state-specific error, not an audio buf error
- Audio status cannot be requested in non-ready states
- The audio buffer cannot be configured in states other than ReadyNoReadsMade
- Using the stop motor command while the motor is already stopped doesn't change states
Additionally, the internal state IDs are used (which distinguish ReadyNoReadsMade and Ready), instead of the state IDs exposed in request error. This makes some of the weird behavior a bit more obvious.
State and error behavior of the seek command was not implemented in this commit.
It is my opinion that nobody should use NKit disc images without
being aware of the drawbacks of them. Since it seems like almost
nobody who is using NKit disc images knows what NKit is (hmm, now
how could that have happened...?), I am adding a warning to Dolphin
so that you can't run NKit disc images without finding out about the
drawbacks. In case someone really does want to use NKit disc images,
the warning has a "Don't show this again" option. Unfortunately, I
can't retroactively add the warning where it's most needed:
in Dolphin 5.0, which does not support Wii NKit disc images.
Pretty much the same optimization we did for AVX, although slightly more
constrained because we're stuck with the two-operand instruction where
destination and source have to match.
We could also specialize the case where registers b, c, and d are all
distinct, but I decided against it since I couldn't find any game that
does this.
Before:
66 0F 57 C0 xorpd xmm0,xmm0
66 41 0F C2 C1 06 cmpnlepd xmm0,xmm9
41 0F 28 CE movaps xmm1,xmm14
66 41 0F 38 15 CC blendvpd xmm1,xmm12,xmm0
44 0F 28 F1 movaps xmm14,xmm1
After:
66 0F 57 C0 xorpd xmm0,xmm0
66 41 0F C2 C1 06 cmpnlepd xmm0,xmm9
66 45 0F 38 15 F4 blendvpd xmm14,xmm12,xmm0
Léo Lam