DrawLine weighted the endpoints by (steps - i) / steps with i running 0 to
steps - 1, which samples each pixel's leading edge. The last pixel therefore sat
a full step short of v1 and the end vertex's values were never used at all.
Visible in gpu/texmtx/prims, whose line strip ends exactly on the bottom-left
pixel: with normal-projected texgen that pixel should carry v3's texcoords
(texel 0, 255) and we produced (1, 254). Working the interpolation out by hand
against the hardware reference pins the correct parameter at (i + 0.5) / steps,
the pixel center - the same rule the triangle and sprite paths now use. Sampling
the endpoint itself is wrong too: that gives (0, 0), because t/q lands on exactly
256 and wraps.
Kept in halves so the color interpolation stays integer.
The textual half of gpu/texmtx/prims now matches the hardware reference exactly.
The test still fails on its screenshot comparison, whose MSE is unchanged to six
decimals - that's a different primitive type, not the lines.
317 pspautotests pass, 0 fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
Two leftovers from the sample point having been a sixteenth of a pixel off
center. They're one commit because they can't be separated: each was
compensating for the other, so applying either alone makes
gpu/filtering/precisionnearest3d fail. I tried both orderings.
1. DrawRectangle advanced ST to the first sample with (minX - entireX1 + 1).
The +1 existed to make up for centerOff being 7 instead of 8; now that minX
already sits at the pixel center it double-counts and pushes the texture
coordinate a sixteenth of a texel too far. Only visible when that lands
exactly on a texel boundary, which is what precisionnearest2d's offset-7 case
constructs: the sprite spans x from -7/16, so u at pixel 0's center is 0.9375
and should sample texel 0, but the extra sixteenth made it exactly 1.0.
2. Removing the +1 exposed the other one. ClipToScreenInternal used a plain
(int) cast, which truncates toward zero - so once the region offset makes the
value negative it rounds the opposite way from the positive case, and from
the through-mode path that computes screenpos directly. The two disagreed by
one subpixel for negative coordinates, which is why the 2D and 3D variants of
the same test failed at different offsets, 7 and 8. floorf is what the +0.375
nudge was always meant to pair with, and it makes both paths agree.
Now passing: gpu/filtering/precisionnearest2d, promoted to tests_good since it
passes on Vulkan too. precisionlinear2d and precisionlinear3d also start passing
on software but still fail on the hardware backends, so they stay in tests_next
with a comment saying so, as does gpu/primitives/continue from the last commit.
317 pspautotests pass, 0 fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
TriangleEdge::Start's comment says "Start at pixel centers", but centerOff was
(SCREEN_SCALE_FACTOR / 2) - 1, i.e. 7 of 16 - a sixteenth of a pixel short of the
real center at 8. DrawRectangle and DrawPoint had the same off-by-one.
Found via gpu/texmtx/uvs, which draws a quad whose UVs span 0..2 across 256
pixels, so one texel is half a pixel and the error is exactly visible: the PSP
samples texel 2x+1 and we sampled 2x. The V axis happened to land a quarter texel
above an integer, so it rounded to the right answer and only U was visibly wrong,
which is what made this look asymmetric. Confirmed by logging the interpolated s/t
at the corner pixels: our sample point sat at pixel + 0.4375 rather than + 0.5.
Both hardware backends already pass that test, so this was softgpu-only.
Now passing and promoted to tests_good: gpu/texmtx/uvs and
gpu/filtering/precisionnearest3d, both of which also pass on Vulkan.
gpu/primitives/continue now passes on software too, but still fails on Vulkan, so
it stays in tests_next for now.
316 pspautotests pass, 0 fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
TODO list at the top of SoftGpu.cpp covering the remaining UB and the platform
differences we haven't settled yet, with enough detail to act on each without
re-deriving it. Also drops a duplicated GPUCommon.h include in SoftGpu.h.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
GCC defaults to -ffp-contract=fast and Clang to on, so both fuse a*b + c into a
single FMA where the hardware has one; MSVC /fp:precise doesn't contract at all.
On aarch64 FMLA is baseline, so the same source gives different depth, fog and
lighting values on Android/Linux than on Windows-on-ARM - exactly the kind of
same-architecture difference we're trying to eliminate. x86-64 only escapes today
because the SSE4.1 baseline has no FMA, which -march=native or x86-64-v3 (as used
by distro and Flatpak packagers) would undo.
Set per-source so it also covers the Math3D.h scalar operator chains inlined into
these TUs, which is where it actually matters. MSVC needs nothing.
Note this only covers the CMake build, which is what the shipping Android build
uses. The legacy android/jni ndk-build and libretro/Makefile.common have no
per-file mechanism, so they'd need it applied globally - left for the wider GPU/
evaluation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
The SSE4 and NEON versions summed pairwise, the scalar Dot() sums left to right,
and float addition isn't associative. Which one ran depended on the build - the
SSE path excluded 32-bit x86 entirely, so Win32 and x64 disagreed on the same
machine - and on cpu_info.bSSE4_1, so one binary disagreed with itself across two
x86 CPUs.
Not cosmetic: the result is fogdepth, which Clipper compares against v0's to
decide whether to split a rectangle into two draws, so a ULP changed the number
of primitives emitted. A 4-element dot product is not where the time goes.
Also parenthesized the sum in Dot(Vec4) so the association is explicit in the
source rather than left to the reader knowing the grammar.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
Unqualified pow picks up the double overload on platforms whose <cmath> only
drops the C version into the global namespace, which computes the whole thing at
double precision and rounds once at the end - a different result decided by the
standard library rather than by us. std::pow always gets the float overload.
That's only half the problem, so there's a TODO next to it: powf isn't correctly
rounded, so it differs between glibc, musl, bionic, Apple's libm and the UCRT,
and all three callers feed the result into a LightCeil - which turns a one-ULP
difference into a full 1/512 step in the light factor instead of letting it wash
out. The PSP almost certainly uses a fixed approximation of its own; settling
that against the pspautotests rendering tests is a job for once the platform
differences are gone and there's a stable base to compare against.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
OptimizePendingStates sat four lines below the `!tasksSplit_ || waitable_->Empty()`
check that makes touching shared state safe. It memcpys a 71-byte PixelFuncID over
a RasterizerState and swaps drawPixel/samplerID, while worker threads copy those
same entries by value to rasterize from - so a primitive could be drawn with the
new drawPixel against the old pixelID bytes.
Moving it inside the guard costs nothing correctness-wise: skipping a round just
means those draws use the unoptimized function, and the next Drain with an empty
waitable picks up the whole accumulated range.
This does not close the whole race - Add* still ORs into states_[stateIndex_].flags
after pushing, which items dispatched by an earlier Drain can be reading. That one
needs the state entries to become copy-on-write once dispatched, which is a bigger
change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
Vec4<float>::Cast<int>() used _mm_cvtps_epi32 under SSE, which rounds using
MXCSR's mode, while NEON's vcvtq_s32_f32 and the scalar (T2)x fallback both
truncate. Same split in Rasterizer's InterpolateI.
That's on depth interpolation, so the differing values are written to the depth
buffer and then compared - a one-LSB difference can flip a later GE_COMP_EQUAL
pass and change a whole surface's visibility, not just a shade. Following MXCSR
also meant anything that left a non-default rounding mode in the render thread
would have changed rasterized output.
Truncation is what two of the three paths already did, so SSE moves to match.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
The SSE, NEON and scalar versions summed the components in three different
orders, so Length() alone differed by architecture. Worse, the SSE normalize
used _mm_rsqrt_ps with no Newton step - about 12 bits - where NEON used vrsqrte
plus two refinement steps and the generic path used exact sqrtf. Three accuracy
tiers for one function, and the SSE2-vs-SSE4.1 choice was made from cpu_info at
runtime, so one binary gave different answers on two different x86 CPUs.
This isn't shading-only: the results feed environment-map texture coordinates via
GE_PROJMAP_NORMALIZED_NORMAL and Lighting's GenerateLightST, so it moves UVs.
sqrtf and division are correctly rounded per IEEE-754, so a single scalar version
is bit-identical everywhere. The horizontal sums being removed were never much
faster than the three multiplies they replaced. The useSSE4 parameter is kept so
the call sites don't churn, but it no longer selects anything.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
Jit_GetTexelCoordsQuad converted s*w*256 with CVTPS2DQ, which rounds. The C++
reference casts with (int), and the nearest JIT paths use CVTTPS2DQ - the quad
path just never got converted when the others did. The JIT's sample point sat up
to 1/512 texel further along than the interpreter's, so roughly one pixel in
sixteen picked a different frac_u/frac_v, and at exact texel boundaries a
different texel.
That mismatch is visible wherever the two paths coexist: x86-64 desktop runs the
JIT, 32-bit x86 and UWP have no sampler JIT at all, and even within one x86-64
session the first draws with a new SamplerID run the C++ path while later ones
run the JIT. CVTPS2DQ also honors MXCSR's rounding mode, so anything that left a
non-default mode in the render thread would have changed rasterized output.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
Expand did `height += ((int)base - (int)newBase) / (stride * bpp)`. The right
operand is unsigned, so when newBase was above base the negative difference
converted to ~4 billion before the division and height wrapped. After that,
HasPendingWrite()'s `start >= base + height * strideBytes` early-out was taken
for every query, so the binner stopped noticing that a draw textures from the
framebuffer it's writing - skipping the flush and leaving maxTasks_ high, which
makes a self-sampling draw depend on which worker thread got there first.
Reachable without exotic state: scissor changes mark BINNER_RANGE dirty without
forcing a flush, so drawing with the scissor top at y=0 and then moving it down
is enough. Handle both directions explicitly instead.
ClearDirty was missing the bounds clamp its twin MarkDirty has. start is masked
to [0, 2047] but bytes isn't bounded - IsVRAMAddress accepts the whole mirrored
8MB window - so a large guest framebuffer near the top of VRAM runs the loop off
the end of vramDirty_[2048] and writes into whatever follows it. Only active with
frameskip enabled, but then it runs every flip.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
The jitted GE_LOGIC_NAND ANDed into temp1Reg, which is uninitialized at that
point and whose result is never read - leaving just the NOT, i.e. COPY_INVERTED
rather than ~(new & old). Every bit where new is 1 and old is 0 came out
inverted. It differed between x86-64 (which has the pixel JIT) and ARM64/UWP/x86
(which don't), and even within one x86-64 run, since GetSingleFunc falls back to
the C++ path when it has to queue a compile.
Subtractive blending clamped to 0 on SSE and in the JIT (PSUBUSW) but not on
NEON (vqsubq saturates at INT_MIN) or the generic path. Normally invisible
because ToRGB() clamps at the end - except the caller adds the dither value in
between, so an underflowing dithered pixel came out up to 7/255 brighter on x86
than on ARM64. Clamp in all paths.
stencil << 24 is signed overflow for stencil >= 128, which is the common case,
not an edge case. Cast to u32 - SetPixelStencil already does this correctly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
Rasterizer.cpp: the secondary color fallback does `prim_color[i] = ` where the
SSE and NEON branches add, so the base/texture color is thrown away and the
triangle renders as pure specular. Introduced by 250abe0d56 (Loongarch64 build
fixes) changing one character; the structurally identical block in DrawRectangle
still has the `+=`. Live on ARM32, LoongArch64, RISC-V64 and anything else that
isn't SSE2 or ARM64.
Lighting.cpp: IsLargerThanHalf's scalar path assigns instead of accumulating in
its loop, so it returns only `v[2] > 1` and ignores the other components, and the
NEON path computes a max where SSE computes a sum. All three disagreed. The
question being asked is "is this color factor non-zero" - the test this replaced
in fcc3b7684e was `!(colorFactor == ones)` - and since LightColorFactor produces
2*c+1, every component is >= 1 and the sum of four is >= 4, which is why the SSE
sum > 4 is the correct one. Made the other two match it.
Getting this wrong doesn't shift a shade, it enables or disables a whole light:
x86-32 and ARM64 were switching lights off that x86-64 left on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
LoadZIMPtr() writes width[]/height[]/image[] as arrays (one entry per
mip level, up to ZIM_MAX_MIP_LEVELS) whenever the file has
ZIM_HAS_MIPS set, per its documented contract - but this caller passed
plain scalar locals. A texture-replacement .zim file with that flag
set caused multiple out-of-bounds stack writes. Now passes properly
sized arrays and only uses level 0, matching the existing "we don't
support ZIM mips yet" behavior. Also fixes a pre-existing leak of
image[0] on the "changed since header read" error path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
Add unittest/TestTextureReplacer which creates a fictive texture pack
(textures.ini with readable invented hashes plus real PNG files), loads
it via the replacer, and verifies lookups, filtering, hashranges, mip
levels, and missing/ignored entries.
To make the replacer runnable outside the emulator:
- The constructor now accepts a null DrawContext (formats just default
to unsupported).
- FindReplacement/FindFiltering use the replaceEnabled_ member instead
of the global config.
- Added TextureReplacer::LoadPackForTesting() to load an ini from a
path directly.
Vuln 17: ReplacedTexture::LoadLevelData let a KTX2/DDS file at a higher
mip level resize the shared data_ vector to its own (attacker-controlled)
mip count, so data_[mipLevel + i] indexed out of bounds and the KTX2
branch resized a different element than it wrote to. Disallow mixing
image formats across mip levels, cap the container mip count, and resize
the same element that is used as the transcode destination.
Vuln 18: DecodeTextureLevel only validated the start address for
non-DXT textures, so guest-controlled w/h/bufw could drive reads past
mapped RAM. Validate the needed range like the DXT path does and clamp
the height; ReadIndexedTex now takes the clamped w/h.
Texture pack filenames/aliases from textures.ini were used to build read
and write paths with no '..' check, letting a malicious pack read or
write files outside the pack directory.
- LoadIniValues rejects entries with a parent dir component via
HasParentDirComponent.
- ReplacedTexture::Prepare skips such filenames as defense in depth.
- PSPLoaders savestate migration now uses the shared HasPathTraversal
helper instead of inline separator checks.
Mirrors the earlier Common extraction. The old "core" target folded in
all of GPU/ (~200 files) plus a few ext/ files wholesale; Windows
already treats GPU as its own project (GPU.vcxproj), so GPU/CMakeLists.txt
splits that out too. GPU has a genuine two-way dependency with Core
(Core/System.cpp calls GPU_Init(), GPU/* calls back into Core for
Memory/Config/CoreTiming/etc), so GPU is a CMake OBJECT library: its
object files are always included wherever consumed instead of being
lazily pulled from an archive, avoiding the GNU ld single-pass
archive-ordering problem a two-way STATIC dependency would hit.
Also fixed a few library misattributions discovered while tracing what
each file actually uses:
- GlslangLibs (glslang/spirv-cross) moved from Core to Common, since
it's Common/GPU/ShaderTranslation.cpp and VulkanContext.cpp that
call into it directly. It only worked before because Core happened
to always be linked after Common.
- ZSTD and OPENGL_LIBRARIES/X11_LIBRARIES moved from Core to GPU,
matching where they're actually called (GPU/Debugger/Record.cpp and
Playback.cpp for ZSTD, GPU/GLES for raw gl*() calls).
- GPU also needs Ext::Snappy directly (Playback.cpp calls
snappy_uncompress) and the libretro-common include dir under
LIBRETRO, both previously inherited for free by accident.
Also fixed USE_DISCORD's add_compile_definitions ordering: it was
being defined after ppsspp_ui's add_library call, so the UI target
never actually saw it on non-MSVC platforms.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSNaZnHCjmryS3ziVN9gZU
