This reverts commit e140516130. This assert triggers for AX and AXWii uCode games (including the Wii System Menu) for various addresses that seem to be 4-byte aligned. Worse still, if the DSP thread is in use (i.e. for DSP LLE recompiler, but not for DSP LLE interpreter), Dolphin completely hangs after the panic alert. Perhaps the data DMA has fewer restrictions compared to the instruction DMA?
The change to DSPTool (e391a28102) has not been reverted, as it still fixes broken behavior for DSPSpy at -O0 on real hardware.
I haven't tested this extensively on real hardware, but I do know that bad things happen if the address isn't properly aligned, and libogc says it should be 32-byte aligned.
This is the behavior in the x64 and ARM64 vertex loaders. I don't know if it makes sense (the whole skipped vertex system seems jank, but several games behave incorrectly without it).
This generated a warning on GCC about the operation being potentially undefined (-Wsequence-point). I'm not sure if that was actually the case, but either way it is a mistake.
Before, it was also compiled on ARM builds, but since it was unused it wasn't linked (and thus its dependency on the nonexistent x64Emitter didn't cause any link issues).
For some reason, when this is included, the linking step creates a temporary file in %TEMP% with a random name; the file is deleted afterwards and a new random name is used on a later build. Because this file doesn't exist on a later build, curl gets re-linked each time, and then all of the projects that depend on curl also get re-linked. This adds around 10 seconds to the build time even for small changes.
To make things worse, I don't think libcurl.rc does anything useful since we statically link curl; I believe the metadata contained in it only applies when building a dll. (It does seem to be included in curl.lib, but gets discarded when linking Dolphin.exe.)
See Build\x64\Release\curl\curl.tlog\Lib-link-cvtres.write.1.tlog for the log that shows this path (the file is also mentioned after setting Tools -> Options... -> Projects and Solutions -> Build and Run -> MSBuild project build output verbosity to diagnostic, but not in a useful way).
Fixes incorrect logspam when the buffer needed to be reset on flushes (which we already were doing, but 52feed04db moved it to after the check was made). This is https://bugs.dolphin-emu.org/issues/10312.
I also converted it to an assert, as if this does happen, things are going to render incorrectly, so we want to make it obvious.
This was added in #10394 for both the hardware and software backends to work around an issue with Mario Kart Wii, Fortune Street, and Baten Kaitos. However, it seems like the software renderer handles blending well enough that we don't need this (and in any case, it's easy to change blending in the software renderer).
Some experimentation with #11387 (not pushed) showed that the software renderer's logic would also produce correct results on the hardware backends with this hack removed, but would require fbfetch (currently); if a better solution is found the hack can also be removed from the hardware backends.
Otherwise, texelFetch() will use an out-of-bounds layer for game textures (that have 1 layer; EFB copies have 2 layers in stereoscopic 3D mode), which is undefined behavior (often resulting in a black image). The fast texture sampling path uses texture(), which always clamps (see https://www.khronos.org/opengl/wiki/Array_Texture#Access_in_shaders), so it was unaffected by this difference.
This affected the memory and registers widgets (and possibly others). I'm pretty sure it regressed in 5f629abd8b.
The SetCodeVisible line is a new fix, but the equivalent already existed in the memory widget.
The call to analyzer.Analyze breaks when it attempts to read an instruction, as it eventually tries to read memory when Memory::m_pRAM is nullptr. Trying to read when execution is not paused in general seems like a bad idea (especially as analyzer.Analyze uses PowerPC::TryReadInstruction which can update icache - this is probably still a problem).
This regressed in 0a906f553f, I think (though I haven't confirmed it). Mario Tennis and Luigi's Mansion both use these for some reason (as far as I can tell, the data isn't actually used; it's just extra data included for no reason)
DataReader is generally jank - it has a start and end pointer, but the end pointer is generally not used, and all of the vertex loaders mostly bypassed it anyways.
Wrapper code (the vertex loaer test, as well as Fifo.cpp and OpcodeDecoding.cpp) still uses it, as does the software vertex loader (which is not a subclass of VertexLoader). These can probably be eliminated later.
0e02ddcf52 removed separate logic for tiled versus non-tiled EFB peek caches, and as part of that made it so that color peeks updated the frame access mask even when a non-tiled cache is in use. However, the same change was not made for depth peeks. I'm not sure if this affected anything in practice.
`ImGui::GetIO` performs an assertion that a context exists, and if one doesn't then things will likely crash. Unfortunately this crash is hard to consistently reproduce.
I originally added these in 2b1d1038a6, for both the TPipelineFunction and the size. The size was moved into the header in fdcd2b7d00 (making the size functions obsolete), but it seems that the functions themselves are no longer needed now.
I think I didn't use this approach before because it would have required ComponentFormatTable and ComponentCountRow to be templated, which would end up resulting in lines that were too long and thus wrapped in awkward places. (I *think* they didn't get inferred properly.) Now that we only need TPipelineFunction, the templating is not needed, and this ends up being a more readable version of the version with the wrapper functions.
The old calculation was stride * (max_index + 1), which fails if stride is less than the size of a component (for instance, if float XYZ positions are used, and the stride was set to 4 (i.e. sizeof(float)) instead of 12 (i.e. 3 * sizeof(float)), it would be missing the last 8 bytes of the final element in the array. Or, if stride was set to 0, then no bytes would be recorded at all (though that's not a useful configuration so it's unlikely to actually exist).
I'm not aware of any games affected by this issue.
This should fix recording the wall in the staircase leading to the basement in Luigi's Mansion (though I haven't tested it, as I don't own a copy of Luigi's Mansion). This uses NormalIndex3, and the index for the normal vector (generally 0x02XX or 0x01XX) there is always lower than the tangent or binormal (generally 0x07XX). Other games seem to usually have a similar range of indices for the normal, tangent, and binormal, so this issue wouldn't affect them.
In most cases, games will use the same type for all vertex components (either Index8 or Index16 or Direct). However, RS2's deflection towers use Index16 for the texture coordinate and Index8 for everything else, meaning the texture coordinates were recorded incorrectly (the first byte was used, so only indices 0 and 1 were recorded instead of 0 through 0x0192). Worse still, some background elements in RS2 use direct positions but indexed normals or texture coordinates, and those would not be recorded at all.
This is a regression from b5fd35f951.
`count` is the number of stereo samples to write (where each stereo sample is two shorts), while `BUFFER_SIZE` is the size of the buffer in shorts. So `count` needs to be multiplied by `2`, not `BUFFER_SIZE`. Also, when this check was failed, the previous code just clobbered whatever was past the end of the buffer after logging the warning, which corrupted `basename`, eventually resulting in Dolphin crashing.
This affected Datel's Wii-compatible Action Replay, which uses a block size of 2298, or 18384 stereo samples, which is 36768 shorts, which is bigger than the buffer size of 32768. (However, the previous commit means that only one block is transfered at a time, eliminating this issue; fixing the bounds check is just a general safety thing instead of an actual bugfix now.)
Nothing currently uses it. It could theoretically be replaced with fmt support, but I don't think the LOG_VULKAN_ERROR macro is that useful and it'd be better to replace it with regular logging instead.
Rather than makring some parts of VertexLoaderManager dirty in some places and some in others, do it all in VideoState. Also, since CPState no longer contains pointers/non-CP data after d039b1bc0d, we can just use p.Do on it instead of manually saving each field.
This existed in the initial megacommit (though I don't know why) as IO_SIZE. It was used in Memmap's Init() to compute totalMemSize, but I don't know if it actually did anything then. That use was removed in 2d0f714546, but the constant persisted until cc858c63b8, when it became a static variable.
This was added in 385d8e2b15, but became somewhat redundant with Do in 4c7bbd96e4, and completely redundant now that std::is_trivially_copyable_v is well-supported.
Before, only the symbols box would update. However, if you edit the symbol of a function in the call stack (which seems like something that would happen reasonably often while debugging), the call stack would be out of date until it was updated by clicking on it. Callers and calls were more of an edge case; for them to be out of date, you would need to right-click on an instruction in a function other than the one containing the currently-selected instruction (though it would also affect recursive functions).
Texture dumping can already be done using VideoCommon's system (and in fact the same setting already enabled *both* of these). Dumping objects/TEV stages/texture fetches doesn't currently have an equivalent, but could be added to the FIFO player instead.
This currently fails for direct with NormalIndex3 enabled (see https://bugs.dolphin-emu.org/issues/12952). The goal of this test is to be able to confidently say that that bug has been fixed.
We have one that does a similar thing, but only to measure speed and uses indices. This one verifies accuracy (and uses the largest possible input size by using direct components).
* 'hangle' was a typo
* Light colors include an alpha value, so they should be 8 characters, not 6
* The XF command format adds 1 to the count internally (so 0 is one word), but we need to subtract that back to produce a valid command
* XFMEM_POSTMATRICES was calculating the row by subtracting XFMEM_POSMATRICES (POS vs POST), resulting in incorrect row numbering
It stores both the konst selection value for alpha and color channels (for two tev stages per ksel), and half of a swap table row (there are 4 total swap tables, which can be used for swizzling the rasterized color and the texture color, and indices selecting which tables to use are stored per tev stage in the alpha combiner). Since these are indexed very differently, the old code was hard to follow.
The masking was incorrect. This affects the main menu of The Last Avatar, though that menu also relies on copy filter functionality that is not correctly handled in the software renderer so the difference is not obvious; that game shuffles textures across all indices for some reason, so this issue would presumably result in subtle flickering.
Pokechu22