feat(smx): per-pad light ownership, idle pad lights option, and frame dedup#617
Open
fchorney wants to merge 7 commits into
Open
feat(smx): per-pad light ownership, idle pad lights option, and frame dedup#617fchorney wants to merge 7 commits into
fchorney wants to merge 7 commits into
Conversation
fchorney
force-pushed
the
fc/smx-idle-pad-lights
branch
2 times, most recently
from
d309ca4 to
aa5c88c
Compare
fchorney
changed the title
The SmxBgPack and SmxJudgePack rows hold a single static placeholder choice; the real list (Default plus discovered packs) is provided dynamically by the options layout. The screen-init sync went through set_choice_by_id, which clamps the index to the static choice list length, forcing the displayed selection to 0. The rows therefore always showed Default when re-entering the options screen, even though the saved config and the actual pad lighting kept the chosen pack. Write the index directly via get_choice_by_id_mut instead, matching the noteskin and software-renderer-threads rows that use the same dynamic-choices pattern.
CanBeEmpty exists so a pack can say a role should never have an animation. The registry honored that for the pack-level chain (declared Fallback pack, automatic common fallback), but the app's screen resolution then tried the default role, so a pack that declared CanBeEmpty=gameplay while supplying its own default gif still showed that default gif during gameplay. Walk the role candidates (grade-specific results roles, the screen role, then default) in order and stop dead at any candidate the selected pack declares under CanBeEmpty without supplying: no animation resolves at all. A supplied gif still wins over its own declaration, and per-song / per-pack scoped gifs sit above pack policy and are unaffected. Also aligns the docs with the actual empty behavior: during gameplay the game keeps LED ownership so an empty background is solid black; on other screens an empty pad is handed back to the pad firmware's built-in lighting.
The lights worker sent the full 1350-byte frame to the SDK every 33ms tick even when nothing changed, so a static background (or an all-black one) streamed at 30Hz for no visual benefit. Compare the final wire bytes (after the user brightness scale, so a brightness change alone still sends) and skip identical frames. Identical frames are still re-sent every 400ms: the pad firmware falls back to its built-in auto lights after autoLightsTimeout without receiving a lights command (a pad-config value in 128ms units, default ~896ms) and that fallback cannot be disabled, so holding a static frame requires periodic refreshes. The suppression state resets on every active transition, since an identical-looking frame must still be re-sent to take the LEDs back from firmware content.
When pad gif lighting has nothing to display for the current screen (the none pack, or a role a pack leaves empty), the pads reverted to the firmware's built-in lighting. Some setups want them dark instead. Add a machine option to the StepManiaX options page, Idle Pad Lights: Firmware (default, current behavior) releases idle pads to the pad's stored idle and step animations; Black keeps the lights worker active so the pads hold solid black, with panel press animations still playing on every screen. Screens that drive the pad LEDs themselves (Init, Test Lights, pad assignment, the SMX options assignment preview) always release the pads regardless of the option. Holding black costs almost nothing on the wire: the worker's duplicate suppression drops the static frame to the 400ms firmware keepalive. Persisted as SmxIdleLightsBlack in deadsync.ini with load/save round-trip coverage; docs describe the empty-pad behavior per mode and correct the old claim that an empty role always showed solid black.
The panel worker held one global `active` flag and sent one frame covering
both pads. Since each player picks their own gif packs, one pad routinely
resolves a background while the other resolves nothing: the worker went
active for the first and then streamed the second a black frame plus the
400ms keepalive, so a pad set to the `none` pack never returned to its
firmware lighting and still played press gifs. Idle Pad Lights: Firmware
could not take effect on either pad unless both were empty.
Decide ownership per pad:
- `worker_active` and the worker's `active` become `[bool; PADS]`, with
`want[pad] = gameplay_active || idle_black || backgrounds[pad]`.
- `Ev::Active` carries a pad. Releasing one clears only that pad
(`clear_pad`) and calls `reenable_auto_lights_for_pad`; the other pad
keeps animating.
- `LightsTx` dedups and keepalives per pad, and masks unowned pads out of
the send, so nothing reaches a released pad and its firmware resumes.
- `on_raw_panel` gates on that pad's own ownership, so a released pad
shows the firmware's step lighting rather than our press gif.
Gameplay still claims both pads unconditionally: a judgement effect can fire
on either at any moment. Both pads' commands still ride one queued frame, so
driven pads stay frame-synchronized and releasing one neither stalls nor
skips the other's animation.
Releasing a pad deliberately sends no black frame. A frame sent after the
`S 1` command would re-disable the auto-lighting it just asked for.
Needs rustmaniax-sdk 2.1.0 for `set_lights_for_pads` and
`reenable_auto_lights_for_pad`.
Gameplay claimed both pads unconditionally, on the reasoning that a judgement
effect can fire on either at any moment. That only holds if the pad has
judgement gifs. With the judge pack set to `none` nothing can ever fire, so
the pad was owned, composited to solid black, and streamed with the keepalive
for the whole song: a dead black pad, and no firmware step lighting, even
with Idle Pad Lights on Firmware. Menus released the pad correctly, so the
behaviour changed on entering gameplay.
Own a pad only when it has something to draw:
want[pad] = idle_black
|| backgrounds[pad].is_some()
|| (gameplay_active && judgement_gifs[pad].has_any())
Factored into `wants_pad` so `activate` and `sync_worker` cannot disagree, and
`set_judgement_gifs_for_pad` now re-syncs, since a pack change can claim or
release a pad. `has_any` destructures `JudgementGifs`, so a new animation slot
is a compile error rather than a silently missed one.
Black still overrides and holds an empty pad dark. A pad with a real judgement
pack is still claimed for the whole song and shows black between events.
fchorney
force-pushed
the
fc/smx-idle-pad-lights
branch
from
aa5c88c to
930b541
Compare
Contributor
Author
Heads up:
|
Picks up the fix for a race in ReenableAutoLightsForPad: an already-queued lights frame could land after re-enabling auto lights for a pad and immediately re-disable it. See fchorney/rustmaniax-sdk#13.
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
1 participant
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.
Summary
Four related changes to SMX pad lighting, ending with per-pad ownership. Requires
rustmaniax-sdk2.1.0 (published to crates.io).Rebased onto current
main, including the shell/app-runtime refactor:SmxPanelDrivernow lives indeadsync-smx::gameplay_driverandJudgementGifsindeadsync-smx::panel_fx, so these changes were re-applied to their new homes rather than to the oldsrc/app/smx_panel_fx.rs.Commits
perf(smx): suppress duplicate light frames with a firmware keepaliveThe worker re-sent an identical frame 30 times a second. Now it compares the final wire bytes (after the brightness scale) and suppresses duplicates, re-sending only every 400ms to hold the frame. The pad firmware reclaims the LEDs after
autoLightsTimeout(default ~896ms) with no way to disable that, so a static frame needs a periodic refresh; 400ms keeps a wide margin. A static frame's USB traffic drops from 30 sends/s to 2.5.feat(smx): idle pad lights option, firmware or blackNew machine option on the StepManiaX page (visible with Panel Lights on). Firmware (default) releases an idle pad to its built-in lighting; Black keeps the LEDs and holds the pad dark, which also makes press gifs work on every screen. Ini key
SmxIdleLightsBlack, with a round-trip test.fix(smx): own pad lights per pad, not globallyThe worker held one global
activeflag and sent one frame covering both pads. Since each player picks their own gif packs, one pad routinely resolves a background while the other resolves nothing: the worker went active for the first and then streamed the second a black frame plus the keepalive, so a pad set to thenonepack never returned to its firmware and still played press gifs. Idle Pad Lights: Firmware could not take effect on either pad unless both were empty.Ownership is now per pad (
[bool; PADS]), andLightsTxdedups, keepalives, and masks per pad. Releasing a pad deliberately sends no black frame: a frame after the release command would re-disable the auto-lighting it just asked for. This is what needed the new SDK API (set_lights_for_pads,reenable_auto_lights_for_pad).fix(smx): don't claim a pad gameplay can never draw onGameplay claimed both pads unconditionally, reasoning that a judgement can fire on either at any moment. That only holds if the pad has judgement gifs. With the judge pack on
nonenothing can ever fire, so the pad was held solid black for the whole song with no firmware step lighting, even with Idle Pad Lights on Firmware. A pad is now owned only when it has something to draw:Behavior
none, Firmwarenone, BlackBoth pads' commands still ride one queued frame, so pads that are both driven stay frame-synchronized.
Testing
Unit tests for the per-pad clear, the keepalive interval, the ownership rule, and the ini round-trip. All verified non-vacuous (each fails when the fix is reverted).
Hardware-validated on the two-pad setup: with both packs on
noneand Idle on Firmware, the pads keep their firmware idle and press animations on every screen including gameplay; with a real judge pack, gameplay draws black plus judgement gifs as before. The SDK lock profiler shows nousb_pollcontention andmain_updateholds flat at 11-17µs average through a song.