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
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
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
Fix#21363https://chat.deepseek.com/share/980xe4c2ue35te8cy2
Performance impact is minimal because sprites that require depth/fog are relatively rare, and the fallback still uses the per-pixel rectangle loop (not the full triangle rasterizer)
