Skip to content

feat: add support for stylus barrel roll - #1120

Open
TrueZhuangJia wants to merge 1 commit into
AlkaidLab:masterfrom
TrueZhuangJia:master
Open

TrueZhuangJia wants to merge 1 commit into
AlkaidLab:masterfrom
TrueZhuangJia:master

Conversation

@TrueZhuangJia

Copy link
Copy Markdown

Added support for stylus barrel roll negotiation & communication

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: AlkaidLab/foundation-sunshine/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 31577eed-a1dd-44cc-8b4d-bde5cdf5e50e
📥 Commits

Reviewing files that changed from the base of the PR and between 59bea47 and a3da96f.

📒 Files selected for processing (9)
  • src/input.cpp
  • src/input_activity.cpp
  • src/pen_barrel_roll.h
  • src/platform/common.h
  • src/platform/windows/input.cpp
  • src/rtsp.cpp
  • tests/CMakeLists.txt
  • tests/unit/test_input_activity.cpp
  • tests/unit/test_pen_barrel_roll.cpp

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
平台抽象层代码(Windows/Linux/macOS)。确保各平台实现一致, 注意 Windows API 调用的错误处理和资源释放。

⚙️ CodeRabbit configuration file

Files:

  • src/platform/common.h
  • src/platform/windows/input.cpp
Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。

⚙️ CodeRabbit configuration file

Files:

  • src/platform/common.h
  • src/rtsp.cpp
  • src/input_activity.cpp
  • src/platform/windows/input.cpp
  • src/pen_barrel_roll.h
  • src/input.cpp
测试文件。验证测试覆盖率、边界情况和断言正确性。

⚙️ CodeRabbit configuration file

Files:

  • tests/CMakeLists.txt
  • tests/unit/test_input_activity.cpp
  • tests/unit/test_pen_barrel_roll.cpp
🔇 Additional comments (11)
src/platform/windows/input.cpp (2)

1916-1917: 倾斜角计算仍使用 pen.rotation,与滚转字段分离是有意设计,但需确认。

pen.barrelRoll 在 passthrough 中会回退到 rotation,因此旧客户端行为不变。倾斜转换继续使用方位角 pen.rotation,这是正确的。无需修改。


1849-1863: CANCEL_ALL 处理正确,但需注意重复销毁的安全性。

取消时先注入取消事件,再销毁设备并清空 penInfo。raw->pen 置空后,后续 CANCEL_ALL 会在前面的 !raw->pen 分支直接返回。~client_input_raw_t 在 pen 为空时不会再次销毁。路径一致,无缺陷。

src/pen_barrel_roll.h (1)

57-71: LGTM!

src/input_activity.cpp (1)

13-13: LGTM!

Also applies to: 25-29

tests/CMakeLists.txt (1)

17-22: LGTM!

tests/unit/test_input_activity.cpp (1)

16-30: LGTM!

tests/unit/test_pen_barrel_roll.cpp (1)

1-83: LGTM!

src/platform/common.h (1)

448-448: LGTM!

src/input.cpp (2)

2174-2191: 入队逻辑符合预期。

长度和 magic 校验在读取扩展字段之前完成,探测器在队列锁内更新。无明确缺陷。


2233-2240: 🩺 Stability & Availability

input_stopped 不会导致后续会话丢弃输入。每次 start 都调用 input::alloc,而 alloc 创建新的 input_t;同一对象停止后,输入入口会拒绝新数据。

取消回调与笔输入也不会在多个 task pool 线程上并发执行。尚未处理的队列任务会发现队列已清空并返回;正在处理的任务会先在唯一 worker 上完成,再执行取消回调。因此,所述竞争风险不成立。

src/rtsp.cpp (1)

1196-1200: LGTM!


Summary by CodeRabbit

  • 新功能
    • 支持识别并传递手写笔的笔身滚转信息,在兼容设备上更准确地呈现笔的旋转方向。
    • Windows 客户端可声明对笔身滚转功能的支持。
  • 问题修复
    • 重置输入时会取消活动的笔输入,避免重置后残留笔迹或按键、鼠标状态。

Walkthrough

新增笔身滚转扩展包的解析、状态探测和转发。输入会话重置时清空队列,并在平台上下文存在时取消全部笔事件。Windows 端使用 barrelRoll 设置笔旋转,并在符合条件的 RTSP 描述中声明该能力。

Changes

笔身滚转输入

Layer / File(s) Summary
扩展包协议与验证
src/pen_barrel_roll.h, src/input_activity.cpp, tests/CMakeLists.txt, tests/unit/test_input_activity.cpp, tests/unit/test_pen_barrel_roll.cpp
新增 40 字节笔扩展包布局、滚转探测器、解码和长度校验。输入活动判定会验证匹配的扩展包。新增单元测试及测试目标。
输入会话中的滚转处理
src/platform/common.h, src/input.cpp
平台笔输入增加 barrelRoll。输入队列验证扩展包并选择滚转值,之后将其传入笔转发路径。批处理会同步复制滚转及保留字段。输入重置会停止接收、清空队列、重置探测器,并在平台上下文存在时请求取消全部笔事件。
Windows 笔输出与能力声明
src/platform/windows/input.cpp, src/rtsp.cpp
Windows 笔输入处理增加取消全部事件的处理,并使用 barrelRoll 设置旋转值。符合条件的 Windows RTSP 描述会添加 penBarrelRoll:1。

Priority: ⬇️ Low

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

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant InputQueue
  participant roll_probe_t
  participant passthrough
  participant pen_update
  InputQueue->>roll_probe_t: select(roll, azimuth)
  InputQueue->>passthrough: 传递笔数据和所选滚转值
  passthrough->>pen_update: 传递平台笔输入
Loading

Suggested reviewers: qiin2333

Merge Risk: ⚪ Minimal · up to a3da9

No actionable issue remains identified for the pen barrel-roll change; it is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a3da9

The extension uses the existing input permissions, validates its fixed-size payload, and preserves legacy pen behavior. No introduced security issue was established. End-to-end identity enforcement and deployed disconnect behavior were not independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A client able to send input through the existing session can influence pen output on the host desktop, including axial rotation. Synthetic pen state and device handles are client-specific, but their output affects the host desktop rather than an isolated tenant surface. The extension does not add a separate privileged execution sink.

Trust Boundaries and Controls

  • observed — Attacker-controlled extension bytes pass exact-size validation before logging, batching, and dispatch. The pen passthrough retains the mouse-input enablement gate and coordinate conversion checks before calling the platform sink. These controls do not independently prove upstream session authentication.

Resilience and Maintainability Implications

  • inferred — Stop-time cleanup is ordered after any currently executing input task by the actual single-worker scheduler. Remaining queued input is cleared and future admission is rejected. Windows cancellation removes the repeat callback and destroys and clears the pen device even when cancellation injection fails. This rejects the suspected simultaneous injection/destruction race in the inspected configuration, without establishing deployed failure behavior.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 8 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确概括了新增手写笔 barrel roll 支持这一主要变更。
Description check ✅ Passed 描述提到手写笔 barrel roll 的协商与通信支持,与变更内容相关。
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 24.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 8 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@qiin2333 qiin2333 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

整体过了一遍(核心路径核对 + 布局断言实测编译 + magic 全量冲突检查),设计是稳的:

  • 魔数 0x5500000B 与现有 SS magic(0x55000001–0A)无冲突;
  • 尺寸不符的扩展包会被 activity tracker 判为 nullopt,在队头(passthrough_next_message 直接 return)和批处理循环(erase)两处被丢弃,dispatch/print 不会越界读;
  • probe 在入队锁内先于 hover 批处理观察每个样本,未激活前注入 azimuth,与改动前行为一致;probe 边界(首包 unknown、legacy 包 reset、变化闩锁)有测试覆盖;
  • input_t 每会话新建,reset 仅在 join() 调用一次,input_stopped 终态化无复用路径;Windows cancel 分支先取消 repeat 任务,设备销毁后可经既有 create-if-null 逻辑自然重建;
  • 端序自洽:header.size = 36(不含 size 字段的 BE)、rotation/barrelRoll 按 LE 编解码,与现有 util::endian::little(packet->rotation) 一致;
  • 主目标 CXX_STANDARD 23 满足 std::byteswap;pen_barrel_roll.h + 单测在本仓库 moonlight-common-c 头 + clang -std=c++23 下编译和 static_assert 全部通过。

风险按严重度列一下:

中(集成可观测性)

  1. pen_wire::magic 尺寸/size 字段不符的包会被静默丢弃,全程无日志。若 voidlink-c 对 header.size 约定或 barrelRoll 端序与主机端不一致,表现将是整场会话笔输入全哑且无任何线索。建议在 evaluate 或入队处对 "magic 命中但 valid_size 失败" 加一条 debug/warning 日志。

低
2. reset() 里的 pen cancel 没有平台门控,所有平台都会执行:

  • Linux legacy_input.cpp:会在会话结束时创建可能从未用过的 uinput 笔设备;且全零 pen_input_t(tilt=0、rotation=0 都非 UNKNOWN)会写出一份 tilt(0,0) 的伪报告;
  • Linux inputtino_pen.cpp:若会话分配过笔设备,cancel 会 place_tool 到 (0,0),宿主端笔/光标跳左上角;
  • macOS 为空实现,无害。
    建议 #ifdef _WIN32 门控(与 SDP 属性一致),或给非 Windows 平台的 pen_update 加 CANCEL_ALL 早退。
  1. static_assert 只防了两个 touchpad 魔数,建议对全部现有 SS_* magic 逐一断言,防止未来新增包类型时撞号。
  2. 收到 legacy SS_PEN 包即 reset probe:若客户端协商后混发 legacy/extended 包,probe 永远不会激活。若客户端保证协商后只发 extended 可忽略,请确认 voidlink-c 端行为。
  3. batch 中 dest_pen->reserved = src_pen->reserved 拷贝了客户端可控的保留字段,当前无人读取;建议保持 0 或不拷,防未来误用。
  4. 新 SDP 属性建议补说明:含义、客户端预期(rotation 字段继续维护 azimuth)、对应 voidlink-c 哪个提交。

需作者确认
7. barrelRoll 按 LE、header.size=36 是主机端假设,和 voidlink-c 的实现对过吗?建议在 PR 描述贴出客户端对应代码引用,方便以后两边对照。

另外目前 checks 只有 CodeRabbit,没看到编译/单测结果;本 PR 动了 Windows 注入路径,合并前建议至少跑一轮 Windows 构建 + 新增的 pen_barrel_roll_unit_tests / input_activity_unit_tests。

@TrueZhuangJia

Copy link
Copy Markdown
Author

整体过了一遍(核心路径核对 + 布局断言实测编译 + magic 全量冲突检查),设计是稳的:

  • 魔数 0x5500000B 与现有 SS magic(0x55000001–0A)无冲突;
  • 尺寸不符的扩展包会被 activity tracker 判为 nullopt,在队头(passthrough_next_message 直接 return)和批处理循环(erase)两处被丢弃,dispatch/print 不会越界读;
  • probe 在入队锁内先于 hover 批处理观察每个样本,未激活前注入 azimuth,与改动前行为一致;probe 边界(首包 unknown、legacy 包 reset、变化闩锁)有测试覆盖;
  • input_t 每会话新建,reset 仅在 join() 调用一次,input_stopped 终态化无复用路径;Windows cancel 分支先取消 repeat 任务,设备销毁后可经既有 create-if-null 逻辑自然重建;
  • 端序自洽:header.size = 36(不含 size 字段的 BE)、rotation/barrelRoll 按 LE 编解码,与现有 util::endian::little(packet->rotation) 一致;
  • 主目标 CXX_STANDARD 23 满足 std::byteswap;pen_barrel_roll.h + 单测在本仓库 moonlight-common-c 头 + clang -std=c++23 下编译和 static_assert 全部通过。

风险按严重度列一下:

中(集成可观测性)

  1. pen_wire::magic 尺寸/size 字段不符的包会被静默丢弃,全程无日志。若 voidlink-c 对 header.size 约定或 barrelRoll 端序与主机端不一致,表现将是整场会话笔输入全哑且无任何线索。建议在 evaluate 或入队处对 "magic 命中但 valid_size 失败" 加一条 debug/warning 日志。

低 2. reset() 里的 pen cancel 没有平台门控,所有平台都会执行:

  • Linux legacy_input.cpp:会在会话结束时创建可能从未用过的 uinput 笔设备;且全零 pen_input_t(tilt=0、rotation=0 都非 UNKNOWN)会写出一份 tilt(0,0) 的伪报告;
  • Linux inputtino_pen.cpp:若会话分配过笔设备,cancel 会 place_tool 到 (0,0),宿主端笔/光标跳左上角;
  • macOS 为空实现,无害。
    建议 #ifdef _WIN32 门控(与 SDP 属性一致),或给非 Windows 平台的 pen_update 加 CANCEL_ALL 早退。
  1. static_assert 只防了两个 touchpad 魔数,建议对全部现有 SS_* magic 逐一断言,防止未来新增包类型时撞号。
  2. 收到 legacy SS_PEN 包即 reset probe:若客户端协商后混发 legacy/extended 包,probe 永远不会激活。若客户端保证协商后只发 extended 可忽略,请确认 voidlink-c 端行为。
  3. batch 中 dest_pen->reserved = src_pen->reserved 拷贝了客户端可控的保留字段,当前无人读取;建议保持 0 或不拷,防未来误用。
  4. 新 SDP 属性建议补说明:含义、客户端预期(rotation 字段继续维护 azimuth)、对应 voidlink-c 哪个提交。

需作者确认 7. barrelRoll 按 LE、header.size=36 是主机端假设,和 voidlink-c 的实现对过吗?建议在 PR 描述贴出客户端对应代码引用,方便以后两边对照。

另外目前 checks 只有 CodeRabbit,没看到编译/单测结果;本 PR 动了 Windows 注入路径,合并前建议至少跑一轮 Windows 构建 + 新增的 pen_barrel_roll_unit_tests / input_activity_unit_tests。

补充 c-module 对应 commit:
TrueZhuangJia/voidlink-c@1afa850
关于验证:
使用voidlink在实际业务场景下进行的测试,测试了新旧笔数据包妆容,断开重连有效性。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants