psyqo: derive GPU display ranges instead of hardcoding them

[nicolasnoble]

Replace the hardcoded GP1(06h)/GP1(07h) display-range magic values with
constexpr helpers derived from the psx-spx formulas: the horizontal range as
X1(0x260) + pixels*cyclesPerPixel, and the vertical range centered on the
middle scanline (NTSC 0x88, PAL 0xa3) plus/minus half the displayed lines.

This reproduces the existing values for 256/320/512/640 and for PAL, and
corrects two latent errors the magic numbers hid: the 368-pixel horizontal
range (was 0xc60, now 0xc70 = 0x260 + 368*7) and the NTSC vertical range,
which displayed 239 lines (16..255) instead of 240 (16..256).

Stacked on #2050 and #2051; rebase once those merge. The 368 and NTSC changes
alter on-screen output and want a check on real hardware.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds a compile-time Layout selection to the psyqo GPU class and refactors display initialization, display-area switching, scissor, and clear-region generation to compute coordinates dynamically based on configuration and the selected layout, replacing prior hardcoded values.

Changes

GPU Layout Refactor

Layer / File(s)	Summary
Layout enum and setDisplayArea declaration src/mips/psyqo/gpu.hh	Adds Layout enum and c_layout constant chosen via preprocessor macros, and declares the private setDisplayArea(bool firstBuffer) method.
Dynamic display range computation in initialize/reinitialize src/mips/psyqo/src/gpu.cpp	Adds helper functions to compute horizontal/vertical display ranges, introduces reinitialize as the setup routine, and has initialize reset counter 1 then delegate to reinitialize.
setDisplayArea implementation and flip() wiring src/mips/psyqo/src/gpu.cpp	Implements setDisplayArea to write Ctrl display-area bits based on c_layout, and updates flip() to call it instead of inline writes.
Layout-aware scissor and clear-region generation src/mips/psyqo/src/gpu.cpp	Updates getScissor, getNextScissor, getClear, and getNextClear to compute rectangle coordinates based on c_layout.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant GPUinitialize as GPU::initialize
  participant GPUreinitialize as GPU::reinitialize
  participant GPUCtrl as Hardware::GPU::Ctrl

  Caller->>GPUinitialize: initialize(config)
  GPUinitialize->>GPUinitialize: reset COUNTERS[1]
  GPUinitialize->>GPUreinitialize: reinitialize(config)
  GPUreinitialize->>GPUreinitialize: compute hDisplayRange/vDisplayRange
  GPUreinitialize->>GPUCtrl: write horizontal/vertical range
  GPUreinitialize->>GPUreinitialize: set m_interlaced, m_height, m_refreshRate

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: deriving GPU display ranges instead of hardcoded values.
Description check	✅ Passed	The description directly matches the change set and explains the derived display-range formulas and fixes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/mips/psyqo/src/gpu.cpp (1)

314-395: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider extracting the layout→(x,y) mapping into a single helper.

getScissor, getNextScissor, getClear, and getNextClear repeat the same c_layout branch structure with subtly inverted offsets (the exact place the Horizontal swap above crept in). A single helper such as bufferOrigin(bool firstBuffer, bool next) returning the {x, y} origin would centralize the invariant and prevent this class of divergence.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mips/psyqo/src/gpu.cpp` around lines 314 - 395, The layout-to-origin
logic is duplicated across GPU::getScissor, GPU::getNextScissor, GPU::getClear,
and GPU::getNextClear, and the Horizontal branch has already drifted out of
sync. Extract the c_layout-dependent buffer origin calculation into a single
helper used by these methods, with one path for current vs next buffer
selection, and update the four call sites to consume that helper so the x/y
mapping stays consistent.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/mips/psyqo/src/gpu.cpp`:
- Around line 314-329: The Horizontal buffer selection in gpu.cpp is using the
same VRAM half as setDisplayArea, so drawing and clears end up on the scanout
buffer. Update the Layout::Horizontal branch in the scissor setup here, and make
the same inversion in getClear, getNextScissor, and getNextClear so the
offscreen buffer is selected consistently. Use the existing Layout::Horizontal
handling and the related helper methods to keep the buffer selection logic
aligned.

---

Nitpick comments:
In `@src/mips/psyqo/src/gpu.cpp`:
- Around line 314-395: The layout-to-origin logic is duplicated across
GPU::getScissor, GPU::getNextScissor, GPU::getClear, and GPU::getNextClear, and
the Horizontal branch has already drifted out of sync. Extract the
c_layout-dependent buffer origin calculation into a single helper used by these
methods, with one path for current vs next buffer selection, and update the four
call sites to consume that helper so the x/y mapping stays consistent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 26dc74a4-0f68-429f-922a-3cf65d76fa4e

📥 Commits

Reviewing files that changed from the base of the PR and between e909cc3 and 5d3b835.

📒 Files selected for processing (2)

• src/mips/psyqo/gpu.hh
• src/mips/psyqo/src/gpu.cpp

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant file and inspect the affected functions with line numbers.
git ls-files src/mips/psyqo/src/gpu.cpp
echo '--- outline ---'
ast-grep outline src/mips/psyqo/src/gpu.cpp --view expanded || true
echo '--- around getDisplayArea / getScissor / getNextScissor / getClear / getNextClear ---'
sed -n '220,420p' src/mips/psyqo/src/gpu.cpp | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 10956

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find call sites to understand whether getScissor / getNextScissor represent current vs next frame buffers.
rg -n "getScissor\\(|getNextScissor\\(|getClear\\(|getNextClear\\(|setDisplayArea\\(" src/mips/psyqo/src -g'*.cpp' -g'*.h'

Repository: grumpycoders/pcsx-redux

Length of output: 850

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect layout definitions and any docs/comments describing buffer ordering.
rg -n "Layout::Horizontal|VerticalSwitch|firstBuffer|double buffer|buffer" src/mips/psyqo/src include src/mips/psyqo -g'*.cpp' -g'*.h' -g'*.md'

Repository: grumpycoders/pcsx-redux

Length of output: 21622

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the layout-related declarations around GPU::Configuration and any comments on frame selection.
rg -n "enum class Layout|struct Configuration|firstBuffer|setDisplayArea|getNextScissor|getNextClear" src/mips/psyqo/src/gpu.cpp src/mips/psyqo/src/*.h src/mips/psyqo/include src/mips/psyqo -g'*.h' -g'*.cpp'

Repository: grumpycoders/pcsx-redux

Length of output: 4698

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "enum class Layout|struct Configuration|firstBuffer|setDisplayArea|getScissor|getNextScissor|getClear|getNextClear" src/mips/psyqo/src/gpu.cpp src/mips/psyqo/src/*.h src/mips/psyqo/include src/mips/psyqo -g'*.h' -g'*.cpp'

Repository: grumpycoders/pcsx-redux

Length of output: 5264

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect docs around GPU parity / double buffering to see how "firstBuffer" is defined.
sed -n '170,220p' src/mips/psyqo/CONCEPTS.md | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 7412

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the cube/torus examples for the intended use of getNextClear and frame parity.
sed -n '110,170p' src/mips/psyqo/examples/cube/cube.cpp | cat -n
echo '---'
sed -n '760,800p' src/mips/psyqo/examples/torus/torus.cpp | cat -n
echo '---'
sed -n '80,110p' src/mips/psyqo/examples/hello-chained/hello-chained.cpp | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 6935

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for explanations of "parity" and "buffer" in the GPU docs/source.
rg -n "parity|firstBuffer|double buffering|display area|draw buffer|current buffer|next frame" src/mips/psyqo/CONCEPTS.md src/mips/psyqo/src/gpu.cpp src/mips/psyqo/examples -g'*.md' -g'*.cpp'

Repository: grumpycoders/pcsx-redux

Length of output: 5495

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Focused read of the GPU code around the parity/flip path and layout helpers.
sed -n '256,395p' src/mips/psyqo/src/gpu.cpp | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 6033

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '175,210p' src/mips/psyqo/CONCEPTS.md | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 6425

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check whether these helpers are used anywhere else or only internally.
rg -n "\bgetScissor\b|\bgetNextScissor\b|\bgetClear\b|\bgetNextClear\b|\benableScissor\b|\bdisableScissor\b" src/mips/psyqo -g'*.cpp' -g'*.h'

Repository: grumpycoders/pcsx-redux

Length of output: 1119

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '768,790p' src/mips/psyqo/examples/torus/torus.cpp | cat -n
echo '---'
sed -n '128,146p' src/mips/psyqo/examples/cube/cube.cpp | cat -n
echo '---'
sed -n '92,104p' src/mips/psyqo/examples/hello-chained/hello-chained.cpp | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 2739

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '180,205p' src/mips/psyqo/CONCEPTS.md | cat -n

Repository: grumpycoders/pcsx-redux

Length of output: 5396

---

Invert the Horizontal buffer selection
Layout::Horizontal should use the opposite x offset here and in getClear/getNextScissor/getNextClear; as written, it targets the same VRAM half as setDisplayArea, so drawing and clears land on the scanout buffer.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mips/psyqo/src/gpu.cpp` around lines 314 - 329, The Horizontal buffer
selection in gpu.cpp is using the same VRAM half as setDisplayArea, so drawing
and clears end up on the scanout buffer. Update the Layout::Horizontal branch in
the scissor setup here, and make the same inversion in getClear, getNextScissor,
and getNextClear so the offscreen buffer is selected consistently. Use the
existing Layout::Horizontal handling and the related helper methods to keep the
buffer selection logic aligned.

Source: Linters/SAST tools