Runtime: SampleTexture casts unbounded or NaN texture coordinates to int, which is undefined behaviour

Open Beginner friendly
  • Is something wrong?
#273 2 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 5 days

Nobody has claimed this yet.

  • #282 by @luisbraganca — closed without merging

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
68/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
computer-graphics, reverse-engineering

Research direction

Open ps2xRuntime/src/lib/gs/gs_cpu_backend.cpp and find GSCpuBackend::SampleTexture, where texUf/texVf are computed then static_cast<int> (point sample and bilinear floor paths). Add a finite clamp (as sketched next to fabsQ) on those texel coords before any int conversion so NaN/huge ST cannot hit conv.fpint UB. Done when both paths go through the clamp, the file still builds, and wrap/CLAMP still runs on bounded ints. Do not rewrite vertices unless maintainers choose that path.

Written by the indexing model from the issue text.

Description

What happens

GSCpuBackend::SampleTexture converts interpolated texture coordinates to int without bounding them first. When a game emits large or non-finite S/T, the conversion is undefined behaviour, not merely a wrong texel.

ps2xRuntime/src/lib/gs/gs_cpu_backend.cpp:

const float invQ = 1.0f / fabsQ(q);
texUf = s * invQ * static_cast<float>(texW);
texVf = t * invQ * static_cast<float>(texH);

and then, by two separate paths:

return samplePoint(static_cast<int>(texUf), static_cast<int>(texVf));   // point sample
...
const int u0 = static_cast<int>(std::floor(sampleU));                   // bilinear
const int v0 = static_cast<int>(std::floor(sampleV));

q is already guarded, and neatly so — fabsQ returns 1.0f whenever std::fabs(q) <= 1e-8, and since std::fabs(NaN) > 1e-8 is false, a NaN q lands there too.

s and t are not guarded at all. A large or NaN s gives a texUf that is non-finite or outside the range of int, and static_cast<int> of such a value is UB ([conv.fpint]). wrapTextureCoordinate then operates on whatever came out, so even the CLAMP/REGION_CLAMP modes cannot save it.

This is not theoretical

PCSX2 merged PR #13970 on 2026-09-04 for exactly this input shape:

This is to prevent graphical glitches in Ridge Racer V (RRV), which draws reflections on cars with large (maybe invalid?) ST coordinates.

Their detection includes a NaN test (m_vt.nan.value), so the bad vertices carry NaN as well as large magnitudes. Scope, from the PR author's own dump run over ~2700 traces: more than 1000 had vertices needing a rewrite, and more than 200 had it on over 5% of draw calls. So this input shape is not one game's quirk.

The decisive detail: the author replayed the RRV dump on a real PlayStation 2 and saw no glitches. The game is not at fault — the hardware simply has a bound that a float rasteriser does not.

The bound, and where it comes from

The GS texture-coordinate path is finite: a signed 16-bit fixed-point value. PCSX2 uses (1 << 15) - 2 = 32766:

const GSVector4 large_val = GSVector4::cxpr(static_cast<float>((1 << 15) - 2)) / tsize;

Their threshold is compared against normalised ST (hence the division by texture size); in SampleTexture the values are already texels, so the same bound is 32766 texels directly.

Suggested fix

A helper beside fabsQ, and one call after texUf/texVf are computed — which covers both conversion paths at once. <cmath> and <algorithm> are already included.

// The GS texture-coordinate path is a signed 16-bit fixed-point value.
// (1 << 15) - 2 is the bound PCSX2 settles on; the -2 leaves room for the
// bilinear neighbour's +1 without overflowing.
constexpr float kGsTexCoordLimit = static_cast<float>((1 << 15) - 2);

float clampTexCoord(float c)
{
    if (!std::isfinite(c))
    {
        return 0.0f;
    }
    return std::fmin(std::fmax(c, -kGsTexCoordLimit), kGsTexCoordLimit);
}

texUf = clampTexCoord(texUf);
texVf = clampTexCoord(texVf);

Builds clean with clang-cl, no new warnings.

What this does and does not do. It removes the UB and bounds sampling to what the hardware can address. It does not claim to reproduce PCSX2's pixels: PCSX2 clamps ST at the vertex, before interpolation, which keeps the DDA gradients sane, whereas this clamps the interpolated value. On a triangle with a single out-of-range vertex the two diverge. The vertex-level fix is the more faithful one, at the cost PR #13970 itself flags — de-indexing and rewriting the vertex queue, with measurable CPU overhead. The two-line version seemed the better default for a runtime; the choice is yours.

Independently of the fix

Two smaller things noticed in the same function, neither touched by the above:

  1. fabsQ returns q, not std::fabs(q), despite its name — so a negative q yields a negative invQ. That may well be intentional (the GS does divide by a signed Q), but the name says otherwise and it cost a second read.
  2. The guard is worth having even where CLAMP is active, because the UB happens at the cast, before wrapTextureCoordinate ever sees the value.

Found while statically recompiling Ridge Racer V (NTSC-U, SLUS_200.02) with PS2Recomp — the game whose reflections prompted the PCSX2 change, so this path is one we expect to reach.
Tested on commit 75d729ce. Host: Windows 11 x64, clang-cl 22.1.3 + Ninja, MSVC toolset 14.51.36231, CMake 4.2.3, -flto=thin.
Not specific to this game, and not specific to Windows or to any toolchain: the undefined behaviour is in the cast.

Dominant language
C++
Stars
3.3k
Forks
149
Avg merge
8h 46m
Merged PRs (30d)
1

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ran-j/PS2Recomp

All issues in ran-j/PS2Recomp

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.