USB: Follow the PCRTC video mode for GunCon2 aim - #14770
Conversation
The per-serial calibration table is applied once by AutoConfigure() and never revisited, but its values are video mode dependent -- the table carries commented-out 480i/480p twins for three discs to prove it. Time Crisis 2 and 3 halve the vertical mode when entering split-screen co-op (640x448 -> 640x224). The range of positions the gun can report halves with it while the frozen scale keeps sending the full range, so aim travels twice as far as it should. Scale the vertical deflection by the ratio between the live PCRTC resolution and the one latched on first use. The ratio is exactly 1.0 for a game that never changes mode, so output is unchanged everywhere else, and the correction is only armed when a table row actually matched -- manually configured guns and unlisted games are untouched. Only the vertical is corrected. The horizontal reading also moves when one of the two display circuits is disabled (Time Crisis 3 reports 320x224 for several frames while transitioning), which is a change in what is being composited rather than in video timing. The ratio is clamped so it can only reduce the deflection. The reference is not saved to the state, so a state made in split-screen latches the reduced height on load; the clamp keeps that inert rather than letting it overshoot by 2x when the game returns to full height.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for submitting a contribution to PCSX2
As this is your first pull request, please be aware of the contributing guidelines.
Additionally, as per recent changes in GitHub Actions, your pull request will need to be approved by a maintainer before GitHub Actions can run against it. You can find more information about this change here.
Please be patient until this happens. In the meantime if you'd like to confirm the builds are passing, you have the option of opening a PR on your own fork, just make sure your fork's master branch is up to date!
There was a problem hiding this comment.
Haven't tested it, but from a brief look at the code:
I don't think you can access g_gs_renderer from the CPU thread like that. For example, if the GS thread decides to reopen when the CPU thread is in GSgetDisplayResolution the local gs variable will become a dangling pointer.
93cae5d to
0894e7e
Compare
chaoticgd
left a comment
chaoticgd
left a comment
There was a problem hiding this comment.
I can't properly test this since I don't have the perhipheral in question, so I'll just dimiss my stale review rather than reviewing again.
Technically those variables should be synchronized e.g. with an std::atomic (if the type passed is large it makes a lock). On x86 cache coherency isn't an issue at least but you are lying to the compiler which can lead to odd behaviour. Granted the existing code doesn't do it.
GSgetDisplayResolution() took a raw pointer from g_gs_renderer and dereferenced it, but guncon2 calls it from the EE thread. If the GS thread reopened the renderer between the null check and the read, the pointer dangled. Store the resolution on the GS thread where it is already computed and read the copy instead. This matches s_last_draw_rect directly above it, which exists for this exact reason and which the same guncon2 call site already reads via GSTranslateWindowToDisplayCoordinates. The snapshot is a relaxed atomic so that the compiler knows it is shared. x86 cache coherency was never the problem, but a plain static would leave it free to cache the value in a register, hoist the load, or split the two components either side of a mode change. Keeping it to one atomic rather than two means width and height always come from the same frame, and the static_assert rejects a non-lock-free fallback. Reported by chaoticgd.
0894e7e to
3c670e9
Compare
4 participants
Description of Changes
The per-serial calibration table is applied once by AutoConfigure() and never revisited, but its values are video mode dependent -- the table carries commented-out 480i/480p twins for three discs to prove it.
Time Crisis 2 and 3 halve the vertical mode when entering split-screen co-op (640x448 -> 640x224). The range of positions the gun can report halves with it while the frozen scale keeps sending the full range, so aim travels twice as far as it should.
Scale the vertical deflection by the ratio between the live PCRTC resolution and the one latched on first use. The ratio is exactly 1.0 for a game that never changes mode, so output is unchanged everywhere else, and the correction is only armed when a table row actually matched -- manually configured guns and unlisted games are untouched.
Only the vertical is corrected. The horizontal reading also moves when one of the two display circuits is disabled (Time Crisis 3 reports 320x224 for several frames while transitioning), which is a change in what is being composited rather than in video timing.
The ratio is clamped so it can only reduce the deflection. The reference is not saved to the state, so a state made in split-screen latches the reduced height on load; the clamp keeps that inert rather than letting it overshoot by 2x when the game returns to full height.
Rationale behind Changes
Fixes split-screen aim scaling with Guncon2 in Time Crisis 2 and Time Crisis 3
Suggested Testing Steps
Did you use AI to help find, test, or implement this issue or feature?
Claude Opus 5 used to implement, fix suggested by stenzek in a thread about the issue. Tested manually.