Skip to content

fix(pad): timestamp fresh controller samples when read - #6337

Open
JJenkx wants to merge 2 commits into
boykopovar:mainfrom
JJenkx:fix/pad-read-timestamps-upstream
Open

JJenkx wants to merge 2 commits into
boykopovar:mainfrom
JJenkx:fix/pad-read-timestamps-upstream

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

Fresh scePadReadState samples and the empty-queue fallback of scePadRead reuse the timestamp of the last input change. Holding a button or stick therefore returns a frozen timestamp across later reads. Stamp these newly generated samples with the current process time; queued samples retain their capture timestamps and order.

Two production lines change. The regression checks advancing timestamps with unchanged buttons/sticks in both paths and verifies that draining the queued sample retains its earlier capture time. Existing latched presses, trigger values and latch reset are preserved. The refresh retains merged #8138 touchpad resolution changes and its controller regressions, including both <cstdio> and <cstring> in the fixture and both functional debt entries. This does not implement hardware sampling cadence or idle sample batching; those limits remain in the library debt file.

Based on main a6646f8464140180351e4c08694dccdd55b2814d, head bdd7a52c1b3b3bb237f15c3a019bd03a8f7b7a64, with two linear topic commits and no merge commits. A local merge-tree check against fetched main 8580e7e214ed44e7c5144565e1a278faf01fc5a7 is clean; the focused test results below belong to the stated base and head.

Open overlap review (2026-10-22): #7881 rewrites both paths for per-port state. Either merge order must keep current-time timestamps for fresh samples and capture-time timestamps for queued entries. Author coordination. The #7881 author reports a local combination passing all 15 pad/user-service tests, including CheckHeldSampleTimes; I have not independently repeated that combined run. #9878 adds disconnected special-port handling and does not change either timestamp site in PadState.cpp. Its CheckSpecialPort test and this PR's CheckHeldSampleTimes are inserted at the same position in GuestPad.cpp; a three-way fixture merge at heads bdd7a52c and 7678df54 conflicts there. Resolve by retaining both function definitions and both calls; neither replaces the other. This is a static overlap review, not a combined build/runtime result. Neither PR is a code dependency. #9770, which included the same ReadState correction, was closed without merging as an accidental fork sync; its implementation is not a dependency.

Tested

Linux x86-64, GCC 16.2.1, Ryzen 9 7950X, exact head above:

  • Configured a fresh Release build and built guest_pad_tests.
  • SDL_VIDEODRIVER=dummy ctest --test-dir build -R '^guest_pad$' -V --timeout 60 --repeat until-fail:3: all three runs pass, no skips. The fixture covers held-input timestamps, captured queue timestamps and the existing touchpad/latch regressions.
  • Negative control: replaced only PadState.cpp with the base revision, rebuilt, and reran the fixture. It failed at scePadReadState returned a stale timestamp for held input. Restoring the candidate source, rebuilding and rerunning passed. Source is clean and still at the tested head.
  • python3 tools/check_conventions.py --base a6646f8464140180351e4c08694dccdd55b2814d and git diff --check: passed during the linear rebase review.

Earlier revisions had broader controller/library checks and independent negative controls for both production lines; those are historical evidence, not tests of this refreshed head. No native game, Windows runtime, full runtime suite, GPU or performance result is claimed for this head.

Pinned validation logs and negative control, SHA-256 264ef40273a4e022f4409e01b6c8c9e02106e6c3896c924b43fedb988ce25686.

The touch-ID ordering agreement in the review remains applicable if #6485 is reopened or resubmitted: merge its prerequisite and touch-ID change first, then retain both captured touch IDs and now for fresh samples. That closed PR is not a current code dependency; queued samples retain their captured IDs and times.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a; controller CPU tests
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 7881 core/libs/prx/libScePad/src/PadState.cpp L237-242, L255-257
PR 9878 core/libs/tests/GuestPad.cpp L312
PR 9919 core/libs/prx/libScePad/src/PadState.cpp L238-239

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 5038 docs/dev/debt/libScePad.md
PR 6044 core/libs/tests/GuestPad.cpp
PR 6879 core/libs/prx/libScePad/src/PadState.cpp
PR 9178 core/libs/tests/GuestPad.cpp
docs/dev/debt/libScePad.md

@Ondra97

Ondra97 commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

I opened #6485 for a separate touch-contact identity bug. The changes meet at the two fresh-sample BuildData calls.

I propose #6485 first, after its shared Windows build prerequisite #6452, then rebasing this timestamp change while retaining both now for fresh sample timestamps and the captured touchIds. Queued samples keep their capture timestamp and IDs.

I manually composed this PR at 279fee87a077f413daa950deb73c2940232f0a3b with the contact-ID sources from 68c8a853 on main 92e50418. Those two Pad source blobs are unchanged in the published #6485 head. On Ubuntu24.04.5/GCC13.3, all four input tests passed; the timestamp regression failed when its two-line timestamp correction was omitted. Restoring the standalone contact-ID sources passed four tests again. This is Linux CPU composition evidence, not a combined Windows, GPU or game result.

Would this integration order work for you?

AI-assisted: yes (Codex).

@JJenkx

JJenkx commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

I agree with the proposed order if the touch-ID change is reopened or resubmitted: #6452, then the touch-contact fix, then #6337 rebased to preserve both the captured touch IDs and now for fresh-sample timestamps. Queued samples must retain their captured IDs and times. I see #6485 is currently closed without merging, so I will not treat its code as present on main. The latest #6337 validation remains independent of that change.

Your four-test Linux composition result is useful evidence; I have not independently repeated that touch-ID combination or claimed a combined Windows result.

@JJenkx

JJenkx commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Temporarily closing to stay within the 20-open-PR author cap while submitting a verified Cocoon startup fix. This timestamp work is preserved at 279fee8 on JJenkx/AnyPS5 branch fix/pad-read-timestamps-upstream; it is not withdrawn on technical grounds. The ordering notes already in the description remain applicable when a slot is available.

@JJenkx JJenkx closed this Oct 16, 2026
@JJenkx JJenkx reopened this Oct 19, 2026
@adriadam10 adriadam10 self-assigned this Oct 19, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 19, 2026
@JJenkx
JJenkx force-pushed the fix/pad-read-timestamps-upstream branch from 279fee8 to 05878ce Compare October 19, 2026 02:24
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 19, 2026
@JJenkx
JJenkx force-pushed the fix/pad-read-timestamps-upstream branch from 05878ce to 90b204c Compare October 19, 2026 16:29
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 19, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 19, 2026
@JJenkx
JJenkx force-pushed the fix/pad-read-timestamps-upstream branch from 90b204c to 554b813 Compare October 19, 2026 20:57
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 19, 2026
@JJenkx
JJenkx force-pushed the fix/pad-read-timestamps-upstream branch from 554b813 to 3ca580f Compare October 20, 2026 17:28
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 20, 2026
@JJenkx
JJenkx force-pushed the fix/pad-read-timestamps-upstream branch from 3ca580f to bb56f95 Compare October 20, 2026 17:30
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 20, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 22, 2026
Current state and an empty-queue read reused the last input-change timestamp while reporting a fresh sample. Timestamp those samples at the read, preserving capture timestamps of queued input. Add a held-state regression covering both APIs and queued chronology. Full integration build, guest_pad and patched_libraries pass; the new regression fails before the change. Sample-time meaning follows device-tested PS5 input research bda3543a; host sampling cadence is still approximate and documented.
@JJenkx JJenkx mentioned this pull request Oct 22, 2026
12 tasks
@JJenkx
JJenkx force-pushed the fix/pad-read-timestamps-upstream branch from bb56f95 to bdd7a52 Compare October 22, 2026 18:38
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 22, 2026
@slugzero

Copy link
Copy Markdown

Author of #7881 here — confirming agreement with the coordination notes. I ran a test rebase combining both branches locally; the joint test suite passes 100% (including CheckHeldSampleTimes). Fine with either merge order: #7881 (comment)

@JJenkx

JJenkx commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for confirming the merge order and reporting the joint 15-test run. I have linked your result in the description and kept it attributed to you. Fresh samples must use now; queued entries must retain their capture times.

I also reviewed the new #9878 overlap: it adds special-port handling, while #6337 only changes the two PadState.cpp timestamp sites. Their fixture additions collide at the same insertion point before CheckRemoteController; whichever lands second should retain both CheckHeldSampleTimes and CheckSpecialPort, including both calls. I have not independently built or run that combination.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants