By not setting a stepSize, stepSize was getting set to the default
value of 0, which is an Int. This later caused a crash when trying to
cast it to Float.
Whenever JitBaseBlockCache::Clear() got called, it threw away the memory mapping for the fast block map and created a new one. This new mapping typically got mapped at the same address at the old one, but this is not guaranteed. The pointer to the mapping gets embedded in the generated dispatcher code in Jit64AsmRoutineManager::Generate(), which is only called once on game boot, so if the new mapping ended up at a different address than the old one, the pointer in the ASM pointed at garbage, leading to a crash.
This fixes the issue by guaranteeing that the new mapping is mapped at the same address.
While both fastmem and the BLR optimization depend on fault handling,
the BLR optimization doesn't depend on fastmem, and there are cases
where you might want the BLR optimization but not fastmem. For me
personally, it's useful when I try to use a debugger on Android and have
to disable fastmem so I don't get SIGSEGVs all the time, but it would be
especially useful for iOS users.
Using GTEST_SKIP instead of just returning from the function shows that
a test was skipped in the test summary. If GTEST_SKIP is called the rest
of the function won't be run, just like with the return.
GTEST_SKIP wasn't available until gtest 1.10, and we updated to 1.12 in
597f8f1b87.
Because CPU thread config changed callbacks are no longer instant,
g_Config.iEFBScale doesn't yet contain the new value when the hotkey OSD
code tries to read it.
Should fix https://bugs.dolphin-emu.org/issues/13343.
There's no reason not to allow this now that these settings are
cleanly integrated into the new config system. (Actually, maybe
we could even have done this before the previous commit...)
This fixes a problem where changing the JIT debug settings on
Android while a game was running wouldn't cause the changed settings
to apply to code blocks that already had been compiled.
Resolve warning caused by using values from two different enums in a
conditional expression which was deprecated in c++20.
The warning in question is clang -Wdeprecated-anon-enum-enum-conversion
and gcc -Wenum-compare.
We had one implementation of this type of data structure in Arm64Emitter
and one in VideoCommon. This moves the Arm64Emitter implementation to
its own file and adds begin and end functions to it, so that VideoCommon
can use it.
You may notice that the license header for the new file is CC0. I wrote
the Arm64Emitter implementation of SmallVector, so this should be no
problem.
Modify PPCSTATE_OFF and PPCSTATE_OFF_ARRAY macros when using GCC to
avoid useless log spam. Specifically, use a consteval lambda with gcc
_Pragma statements to disable the -Winvalid-offsetof warning inside the
macros.
Each successful build (and many failing ones) on the Android buildbot
generates almost 300 cases of -Winvalid-offsetof, resulting in thousands
of lines of log spam per build. In addition to bloating the log filesize
these spurious warnings make it harder to find actual warnings.
These warnings are generated by calls to the macros PPCSTATE_OFF and
PPCSTATE_OFF_ARRAY, which in turn are used by many other macros used by
the JIT. The ultimate cause is that offsetof is only conditionally
supported on non-standard-layout types, which includes the PowerPCState
struct.
To address potential questions of whether there's a better way to handle
this:
The obvious solution would be to modify PowerPCState so that it does
have a standard layout. This is unfortunately impractical.
To have a standard layout a type can only contain other types with
standard layouts. None of the stl containers are guaranteed to have
standard layouts, and PowerPCState contains a std::tuple and std::array.
PowerPCState also contains a PowerPC::Cache and InstructionCache which
themselves contain std:arrays and std::vectors.
Furthermore InstructionCache derives from Cache, and a derived class can
only have standard layout if at most one class in its hierarchy has a
non-static data member, but both classes have such members. Making
InstructionCache have a standard layout would require duplicating all
the functionality of Cache so it no longer derived from it, as well as
replacing the stl containers. This might require having a raw pointer to
said containers, with the manual memory management that implies.
All of that would be much more disruptive than would be justified to get
rid of some warnings (however annoying they might be). This is
compounded by the fact that PowerPCState hasn't had a standard layout
for a long time, if ever, and if the PPCSTATE_OFF macros weren't working
reliably it would have become obvious a long time ago.
As to why I picked the lambda solution over other potential changes:
- Keeping the define as-is and wrapping some gcc #pragmas around it
doesn't work because the pragmas don't get included when the define is
substituted to the call site.
- Keeping the define as a non-lambda expression and using inline
_Pragma() statements would ideally be better and works fine for msvc,
but fails for GCC with "'#pragma' is not allowed here".
- Turning off -Winvalid-offsetof globally for gcc would work, but there
might be other contexts where offsetof is problematic and GCC seems to
be the only compiler warning about it.
This was because the shader uniforms between the pixel and vertex shaders
were willingly left different, to avoid filling the vertex shader with unnecessary
params. Turns out all backends are fine with this except OGL.
The new behaviour is now much more consistent and well explained,
the "default" shaders are the ones that always run, and the non default
ones are the user selected ones (if any).
Turns out that we have two subsystems that want to register CPU thread
callbacks from a different thread than the CPU thread: FreeLookConfig
and VideoConfig. Both seem to happen from the host thread before the CPU
thread starts, so there's no thread safety issue. But ideally, if we
want to allow registering callbacks from threads other than the CPU
thread, we should make registering callbacks actually thread-safe. This
is an unsolved problem for the regular Config system, so I would like to
leave it outside the scope of this PR.
This fixes a problem that started happening in CoreTimingTest after the
previous commit. CPUThreadConfigCallback registers a Config callback
only once per run of the process, but CoreTimingTest calls
Config::Shutdown after each test, and Config::Shutdown was clearing all
callbacks, preventing the callback from running after that.
In theory, our config system supports calling Set from any thread. But
because we have config callbacks that call RunAsCPUThread, it's a lot
more restricted in practice. Calling Set from any thread other than the
host thread or the CPU thread is formally thread unsafe, and calling Set
on the host thread while the CPU thread is showing a panic alert causes
a deadlock. This is especially a problem because 04072f0 made the
"Ignore for this session" button in panic alerts call Set.
Because so many of our config callbacks want their code to run on the
CPU thread, I thought it would make sense to have a centralized way to
move execution to the CPU thread for config callbacks. To solve the
deadlock problem, this new way is non-blocking. This means that threads
other than the CPU thread might continue executing before the CPU thread
is informed of the new config, but I don't think there's any problem
with that.
Intends to fix https://bugs.dolphin-emu.org/issues/13108.
In std::string, you can store strings using any encoding, but in Dolphin
we have decided to use UTF-8. The problem is that if you convert between
std::string and std::filesystem::path using the built-in methods, the
standard library will make up its own assumption of what encoding you're
using in the std::string. On most OSes this is UTF-8, but on Windows
it's whatever the user's code page is.
What I believe is the C++ standard authors' intended solution to this is
to use std::u8string instead of std::string, but that's a big hassle to
move over to, because there's no convenient way to convert between
std::string and std::u8string. Instead, in Dolphin, we have added helper
functions that convert between std::string and std::filesystem::path in
the manner we want. You *always* have to use these when converting
between std::string and std::filesystem::path, otherwise we get these
kinds of encoding problems that we've been having with custom textures.
Fixes https://bugs.dolphin-emu.org/issues/13328.
The previous list had some issues. A lot of variant id's were set to 0x0000. Althought this works for some figures, on a technicallity implemented into the games, they are technically wrong and don't result in exactly the same experience as the real figures. For example, the previous small fry got a "series 1" text in the summon screen. The real small fry does not have this. I also added figure types so I can add seperate generation logic later.
The Kaos element only applies to 3 items. So, I decided to throw it under others since it's not listed as an element in the manual and you can easily search for Kaos
Android interprets char as unsigned char, so comparing with 0 triggers a
tautological-unsigned-char-zero-compare warning.
Casting c to an unsigned char and removing the comparison with 0
resolves the warning while needing one less comparison on all platforms.
Check bytes directly to avoid ambiguity in the disassembly between short
and near jumps, which could hypothetically cause the test to pass when
it shouldn't.
Verify that DXFramebuffer's integer RTV descriptor's cpu_handle has been
allocated before using it, and if it hasn't use the non-integer RTV
descriptor instead. This fixes a Dolphin crash in Twilight Princess, and
possibly other games (Issue 13312).
As an optimization to save space in the descriptor heap, DXFramebuffer's
integer descriptor is only initialized if the given abstract texture
format has different integer and non-integer RTV formats. This
previously wasn't accounted for by GetIntRTVDescriptorArray, which could
cause DX12::Gfx::BindFramebuffer to call OMSetRenderTargets with an
invalid descriptor which would lead to a crash.
Triggering the bug was fortunately rare because integer formats are only
used when blending is disabled and logic ops are enabled. Furthermore,
the standard integer abstract format is RGBA8 which has different
integer and non-integer RTV formats, causing the integer descriptor to
be initialized and avoiding the bug.
The crash started appearing in a2702c6 because it changed the
swapchain's abstract texture format from RGBA8 to RGB10_A2. Unlike
RGBA8, RGB10_A2 has the same integer and non-integer RTV formats and so
the bug can be triggered if the other requirements are met.
Should fix https://bugs.dolphin-emu.org/issues/13178.
Jit64 doesn't need to do this because it stores PC directly into
ppcState instead of first storing it in a register.
Now block link nearcode is back to a length of three instructions.
Unfortunately, the code I'm adding to Jit.cpp ends up being a bit messy
because we need to handle the case of already being in farcode...
Jumping between linked blocks currently works as follows: First, at the
end of the first block, we check if the downcount is greater than zero.
If it is, we jump to the `normalEntry` of the block. So far so good. But
if the downcount wasn't greater than zero, we jump to the `checkedEntry`
of the block, which checks the downcount *again* and then jumps to
`do_timing` if it's less than zero (which seems like an off by one error
- Jit64 doesn't do anything like this). This second check is rather
redundant. Let's jump to `do_timing` where we previously jumped to
`checkedEntry`.
Jit64 doesn't check the downcount on block entry. See 5236dc3.
JosJuice