Repository navigation
fix(gs): saturate STQ texel coordinates before converting them to int - #282
Open
luisbraganca wants to merge 1 commit into
Open
luisbraganca wants to merge 1 commit into
luisbraganca wants to merge 1 commit into
Conversation
GSCpuBackend::SampleTexture converted the perspective-divided texel coordinate to int unbounded, so a NaN or out-of-range S/T was undefined behaviour (point and bilinear paths). Saturate to the 12.4 range the GS uses before the wrap mode, per libgpu2's TexCoordN: positive overflow to 0x7FFF, negative overflow to zero. Adds a frontend-driven regression case. Fixes ran-j#273
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #273.
Problem
GSCpuBackend::SampleTextureconverts the perspective-divided texel coordinate tointwithout bounding it.qis guarded byfabsQ, buts/tare not, so a NaN or out-of-range value reachesstatic_cast<int>on both the point-sample path and the bilinearstd::floorpath — undefined behaviour under [conv.fpint]. Ridge Racer V emits ST values that reach this (see the issue).Fix
A
saturateTexCoord()helper besidefabsQ, applied totexUf/texVfin the STQ branch, which covers both cast sites and every caller (triangle and sprite).The bound follows the hardware rather than being an arbitrary guard. Sony's own GS behavioural model, reconstructed in aap/libgpu2, denormalises each coordinate into 12.4 fixed point in
TexCoordN(src/txm.c) and the filters callWrapU/WrapVonly afterwards, so saturation happens before REPEAT/CLAMP:0x7FFF= 2047.9375 texels0, not-0x7FFFThat model replays Ridge Racer V dumps bit-identically against the original and matches a DTL-T10000 devkit on most captured frames. PCSX2's software renderer uses its own ±32767
ps32bound, which is an emulator representation, not the hardware's — that is where the issue's first number came from.NaN maps to zero deliberately: a host-generated NaN has a negative sign on x86 and a positive one on ARM, so saturating it by sign would make rendering host-dependent.
Test
GS STQ texel coordinates saturate like the 12.4 hardware path before wrappinginps2xTest/src/ps2_gs_tests.cpp, next to the existing CLAMP-mode case. It draws an STQ triangle through theGSfrontend with rawSTbits so NaN/±Inf are injected exactly, and checks 11 cases: in-range wrap unchanged (2046.5 and −1 texels), positive and negative overflow, 1e30, ±Inf, NaN point-sampled, NaN under the linear filter, and the same overflow/NaN on T.Verification
./build/ps2xTest/ps2x_tests: 438 passed, 0 failed.gs_cpu_backend.cppswapped in: 437 passed, 1 failed — only the new case, on the assertions that are deterministic across hosts (positive overflow, negative overflow, NaN in the bilinear weights, T overflow). The other UB cases pass on an ARM host only becausefcvtzssaturates; on x86 they fail too.-fsanitize=undefined,float-cast-overflow: the previous backend reports4e+30 is outside the range of representable values of type 'int'at the point-sample cast andnan …at the bilinear cast; the patched backend reports nothing.Built on Apple Silicon (clang 21,
-DPS2X_ENABLE_FFMPEG=OFF -DPS2X_BUILD_STUDIO=OFF).Not changed, deliberately
TexCoordLin the same model skips the half-texel bilinear offset once a coordinate has saturated;SampleTexturestill subtracts 0.5. Small, separable follow-up.fabsQreturning the signedq(noted in the issue) is left as is.DrawSprite; sprites reach the sameSampleTexturecode.