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.
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.
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.
I initially thought the 0x01 side was both sides (equavalent to just C. However, this turned out to be something I forgot I implemented in my personal interface. 0x01 does not seem to change any colors.
Recently discovered how exactly the last 2 bytes of the J command for timing data
using portmapping with hosting while using traversal server (which is possible by checking the option while under "direct connect" and flipping back to traversal server) causes dolphin to request a mapping to external port 0.
In UPnP a mapping to external port 0 is actually the wildcard, which means that connection requests on all
external ports (that are not otherwise mapped) will be forwarded to the client.
Additionally it seems like using port mapping with traversal server is probably not expected behavior, as the option checkbox disappears when traversal server is used.
Dolphin no longer supports Windows 7, so the fact that there are (were?)
more people who use Windows 7 than who use a GPU that doesn't support
the required feature is no longer relevant.
By misusing Config, this netplay-related code opened up a race condition between Config::OnConfigChanged() and SerialInterface::SerialInterfaceManager::UpdateDevices() that could cause iterator invalidation.
GCAdapter::UseAdapter() reads s_is_adapter_wanted, which gets
initialized by config_guard.~ConfigChangeCallbackGuard(). So we must
wait until after destroying the config guard to know whether we have any
controllers set to GC Adapter.
Expanded the use of the lock mutex already used for loading the player's existing unlock status to guard against races involving the Achievements dialog window reading from data AchievementManager might be in the process of updating. The lock has been exposed publicly and the AchievementsWindow uses it in its UpdateData method, and anywhere else that might modify data used to render that window has also been wrapped with it.
AchievementManager now has a SetUpdateCallback method for providing a single universal callback for anytime something important changes in the achievement state, such as logging in/out, game load/close, or events such as achievement unlocks. AchievementsWindow sets this callback in its own init to its UpdateData method so that the AchievementsWindow gets updated when one of these changes takes place.
This widget is a tab in the AchievementsWindow that displays the player's current achievement progress: which achievements are locked or unlocked, and the progress of achievements that have progress metrics.
This widget displays a header on the AchievementsWindow dialog above the tabs that shows the currently logged in user (if there is one) and the game they are playing (if there is one).
Added some small methods to AchievementManager to expose useful data for displaying in an achievement UI. Also moved a couple things from private to public for the same purpose.
1 ) When first opened, the (user selected) post process shader config widget would print the wrong values on the text label next to int range sliders. For example if the range was from 1 to 6, and the value loaded from the config was 1, the label would print 0 when first opened, to then start showing the correct value once the slider was first moved.
This mirrors the behaviour of the float slider code below:
```auto* const value_box = new QLineEdit(QString::asprintf("%f", m_config_option->m_float_values[i]));```
2 ) The defautl int slider value would also be set wrong on first load, as it was being divided by the slider max instead of the slider step amount (again, just like for the float implementation). This is a mistake I had made with my previous submission.
The ExpansionInterfaceManager::PauseAndLock function does nothing but
call other functions that have no effect.
ExpansionInterfaceManager::PauseAndLock calls CEXIChannel::PauseAndLock,
which in turn calls IEXIDevice::PauseAndLock. None of the classes
deriving from IEXIDevice override PauseAndLock, and the implementation
in IEXIDevice does nothing.
Fixing all the places it's used turned out to be a more complicated task than anticipated. So let's remove this for now so we don't confuse users with cryptic error messages...
To further increase the accuracy of the post process phase, I've added (scRGB) HDR support, which is necessary
to fully display the PAL and NTSC-J color spaces, and also to improve the quality of post process texture samplings and
do them in linear space instead of gamma space (which is very important when playing at low resolutions).
For SDR, the quality is also slightly increased, at least if any post process runs, as the buffer is now
R10G10B10A2 (on Vulkan, DX11 and DX12) if supported; previously it was R8G8B8A8 but the alpha bits were wasted.
Gamma correction is arguably the most important thing as Dolphin on Windows outputted in "sRGB" (implicitly)
as that's what Windows expects by default, though sRGB gamma is very different from the gamma commonly used
by video standards dating to the pre HDR era (roughly gamma 2.35).
Additionally, the addition of HDR support (which is pretty straight forward and minimal), added support for
our own custom AutoHDR shaders, which would allow us to achieve decent looking HDR in Dolphin games without
having to use SpecialK or Windows 11 AutoHDR. Both of which don't necessarily play nice with older games
with strongly different and simpler lighting. HDR should also be supported in Linux.
Development of my own AutoHDR shader is almost complete and will come next.
This has been carefully tested and there should be no regression in any of the different features that Dolphin
offers, like multisampling, stereo rendering, other post processes, etc etc.
Fixes: https://bugs.dolphin-emu.org/issues/8941
Co-authored-by: EndlesslyFlowering <EndlesslyFlowering@protonmail.com>
Co-authored-by: Dogway <lin_ares@hotmail.com>
There's nothing going on with behavior here that would prevent these
from being const qualified.
Also better communicates that this function isn't intended to modify the
given resource pack.
We want to have a low number of instructions between the LDP and the
MADD so that the MADD can start immediately after the LDP finishes
even if we're on a lower-end in-order CPU.
Replace the bool parameter force5bytes in J, JMP, and J_CC with an enum
class Jump::Short/Near. Many callers set that parameter to the literal
'true', which was unclear if you didn't already know what it did.
The base DebugInterface now depends on the Core's CPUThreadGuard, and
utilities in Common shouldn't be depending on Core facilities. So, we
can move this into the core library instead.
Admiral H. Curtiss