docs: refresh to current state and drop .antigravity tooling config#19
Conversation
Bring the repo docs in line with what actually shipped, and remove the .antigravity/ agent-orchestration config (local tooling, not part of the project; now gitignored). The most important fixes correct claims that were no longer true: - HDR/10-bit was listed as implemented. It is not: the AR30/XR30 path keeps only the top 8 of 10 bits with no PQ/BT.2020/tone-mapping, and 16-bit (XR48/AR48) and P010 are unhandled. Now marked in progress (#16) with an honest note. - The detile story said Intel/AMD use a CPU path. The EGL/GLES2 backend is the primary detiler now, with CPU deswizzle as a fallback. - The virtio performance text described a TRANSFER_FROM_HOST + mmap-cache mechanism with unsupported FPS figures. Replaced with the V2 copy / V3 zero-copy DMA-BUF model and the measured per-core CPU figures. Plain virtio still uses TRANSFER_FROM_HOST (direct) / V3 (helper); only the virgl 3D scanout needs GPU EGL readback, which is captured correctly and tracked for production integration in #15. - SECURITY.md now matches the current helper: fd-3 spawn check plus the SO_PEERCRED peer-uid match, O_RDONLY device open, the default-KILL seccomp allowlist with open/openat deliberately excluded, exploit-mitigation flags, and the DoS geometry guard. The AI-assisted-development story (AI_DEVELOPMENT.md, AGENTS.md, the README section) is kept and refreshed, not removed. Versions, internal links, and each document's voice are preserved.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)docs/**/*.md📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/research/**/*.md📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR removes local antigravity agent config content and updates contributor, security, integration, and research documentation to match the current helper, capture, HDR, and RustDesk status. ChangesRepository docs and local agent cleanup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)README.mdmarkdownlint-cli2 wrapper config was not available before execution docs/research/08_reframe_egl_analysis.mdmarkdownlint-cli2 wrapper config was not available before execution Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SECURITY.md (1)
191-199: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReorder the ownership and capability steps.
setcapruns before thechown/chmodblock here, but the sibling install guide does the ownership/mode changes first and only then applies the capability. If the ownership change lands aftersetcap, the file capability can be lost and this example will no longer leave the helper withCAP_SYS_ADMIN.♻️ Proposed fix
- sudo setcap cap_sys_admin+ep /usr/lib/rustdesk/drmtap-helper - - # Recommended on multi-user hosts: restrict who can execute the setcap helper - # to a dedicated group rather than leaving it world-executable. - sudo chown root:rustdesk-capture /usr/lib/rustdesk/drmtap-helper - sudo chmod 0750 /usr/lib/rustdesk/drmtap-helper + # Recommended on multi-user hosts: restrict who can execute the helper + # to a dedicated group before granting the capability. + sudo chown root:rustdesk-capture /usr/lib/rustdesk/drmtap-helper + sudo chmod 0750 /usr/lib/rustdesk/drmtap-helper + sudo setcap cap_sys_admin+ep /usr/lib/rustdesk/drmtap-helper🤖 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 `@SECURITY.md` around lines 191 - 199, Reorder the helper setup steps in SECURITY.md so the ownership and mode changes on drmtap-helper happen before the capability grant. Update the example around the setcap/chown/chmod sequence to match the sibling install guide, keeping the chown and chmod steps first and applying setcap last to preserve CAP_SYS_ADMIN on the helper binary.
🤖 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 `@CONTRIBUTING.md`:
- Around line 116-118: The real-GPU test example is missing the explicit
DRM_DEVICE setting, unlike the vkms example, which can lead to selecting the
wrong GPU on multi-card systems. Update the meson test example in
CONTRIBUTING.md to include DRM_DEVICE or clearly document how the device is
resolved by default, and keep it consistent with the repo guidance referenced by
AGENTS.md.
In `@docs/research/05_api_and_architecture.md`:
- Around line 67-72: Reword the privilege-boundary description in the research
doc to avoid saying DRM master or CAP_SYS_ADMIN is what directly enables reading
other clients’ framebuffers. Update the explanation around the two-process flow,
`drmtap-helper`, `socketpair`, and the framebuffer export path using
`drmModeGetFB2()` and `drmPrimeHandleToFD()`, making clear that the helper only
provides the privilege needed for the export flow.
In `@docs/research/06_github_issues_analysis.md`:
- Around line 226-246: The HDR checklist entry mixes short-form and explicit DRM
fourcc names, which makes the supported formats ambiguous. Update the item in
the research notes to use a single naming scheme consistently, and split the
16-bit formats as needed so `AB48`/`ABGR16161616` are not conflated with `XR48`
or `AR48`; keep the wording around the HDR blocker clear and unambiguous.
In `@docs/research/08_reframe_egl_analysis.md`:
- Around line 5-6: The “ALL GPU tiling formats” wording overstates what ReFrame
demonstrated; revise the claim in the analysis text to limit it to the tested
formats/GPUs/modifiers covered by the EGL path, while still noting that it
informed src/gpu_egl.c as the primary detile path and the CPU backends as
fallback.
- Around line 103-107: The branching description around drmtap_grab_mapped
currently treats all non-linear buffers as eligible for the
gpu_{intel,amd,nvidia}.c CPU fallback, which is incorrect for compressed
modifiers. Update the flow in the drmtap_grab_mapped mapping path to add an
explicit unsupported branch for compressed modifiers such as CCS/DCC: if DMA-BUF
+ EGL is available, route through gpu_egl.c; if only dumb-mmap data is available
for a compressed modifier, mark it as unsupported with -ENOTSUP instead of
falling back to gpu_{intel,amd,nvidia}.c. Use the existing symbols
drmtap_grab_mapped, gpu_egl.c, and gpu_{intel,amd,nvidia}.c to keep the decision
tree clear.
In `@README.md`:
- Around line 92-103: Remove the empty quoted lines in the README blockquotes so
the callouts stay contiguous and markdown lint does not flag MD028. Edit the
quoted sections around the testing status and virgl note text, keeping the
content directly adjacent without blank lines inside the blockquote.
---
Outside diff comments:
In `@SECURITY.md`:
- Around line 191-199: Reorder the helper setup steps in SECURITY.md so the
ownership and mode changes on drmtap-helper happen before the capability grant.
Update the example around the setcap/chown/chmod sequence to match the sibling
install guide, keeping the chown and chmod steps first and applying setcap last
to preserve CAP_SYS_ADMIN on the helper binary.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 89a92448-a016-478a-819e-d86a5b351716
📒 Files selected for processing (30)
.antigravity/agents.json.antigravity/orchestrator.json.antigravity/personas/architect.md.antigravity/personas/coder.md.antigravity/personas/devops.md.antigravity/personas/researcher.md.antigravity/personas/reviewer.md.antigravity/personas/security.md.antigravity/personas/tester.md.github/PULL_REQUEST_TEMPLATE.md.gitignoreAGENTS.mdCONTRIBUTING.mdREADME.mdSECURITY.mdbindings/rust/libdrmtap-sys/README.mdcontrib/integrations/README.mdcontrib/integrations/rustdesk/README.mddocs/AI_DEVELOPMENT.mddocs/README.mddocs/research/00_landscape.mddocs/research/01_wayland_capture_problem.mddocs/research/02_drm_kms_mechanism.mddocs/research/03_permissions.mddocs/research/04_gpu_and_testing.mddocs/research/05_api_and_architecture.mddocs/research/06_github_issues_analysis.mddocs/research/07_potential_adopters.mddocs/research/08_reframe_egl_analysis.mdpatches/rustdesk/README.md
💤 Files with no reviewable changes (9)
- .antigravity/personas/security.md
- .antigravity/personas/reviewer.md
- .antigravity/personas/tester.md
- .antigravity/agents.json
- .antigravity/personas/researcher.md
- .antigravity/personas/coder.md
- .antigravity/personas/devops.md
- .antigravity/orchestrator.json
- .antigravity/personas/architect.md
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: fxd0h/libdrmtap
Timestamp: 2026-06-26T17:29:05.192Z
Learning: Never introduce or claim HDR10/10-bit support as implemented; the AR30/XR30 path is incomplete and 16-bit/P010 are unhandled.
Learnt from: CR
Repo: fxd0h/libdrmtap
Timestamp: 2026-06-26T17:29:05.192Z
Learning: Before starting work, read the research docs and check the gotcha checklist to avoid reintroducing known bugs.
Learnt from: CR
Repo: fxd0h/libdrmtap
Timestamp: 2026-06-26T17:29:05.192Z
Learning: When writing code, do not mix concerns between grab, enumerate, convert, and helper layers.
Learnt from: CR
Repo: fxd0h/libdrmtap
Timestamp: 2026-06-26T17:29:05.192Z
Learning: Use the `goto cleanup` error-handling pattern; never crash or abort.
Learnt from: CR
Repo: fxd0h/libdrmtap
Timestamp: 2026-06-26T17:29:05.192Z
Learning: Commit messages should use `component: short description` in imperative mood and may reference issues with `#NNN`.
🪛 LanguageTool
.github/PULL_REQUEST_TEMPLATE.md
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ...s) - [ ] Gotcha checklist reviewed ([06_github_issues_analysis.md](../docs/research/06...
(GITHUB)
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ... reviewed (06_github_issues_analysis.md) - [ ] CI green: Bu...
(GITHUB)
CONTRIBUTING.md
[uncategorized] ~124-~124: The official name of this software platform is spelled with a capital “H”.
Context: ...sh and pull request runs GitHub Actions: - Build & Test ...
(GITHUB)
docs/research/02_drm_kms_mechanism.md
[grammar] ~184-~184: Ensure spelling is correct
Context: ...ia block-linear. EGL is the primary detile path; a CPU deswizzle survives only as ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/research/05_api_and_architecture.md
[grammar] ~83-~83: Ensure spelling is correct
Context: ... to linear RGBA. EGL is the primary detile path; a CPU deswizzle exists as a fallb...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~85-~85: Ensure spelling is correct
Context: ...simple VMs) are mapped directly with no detile. ### Proposed API (headers) ```c // === drmt...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.1)
docs/AI_DEVELOPMENT.md
[warning] 48-48: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 57-57: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
CONTRIBUTING.md
[warning] 104-104: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 105-105: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 109-109: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 110-110: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/research/08_reframe_egl_analysis.md
[warning] 103-103: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
README.md
[warning] 97-97: Blank line inside blockquote
(MD028, no-blanks-blockquote)
[warning] 103-103: Blank line inside blockquote
(MD028, no-blanks-blockquote)
docs/research/02_drm_kms_mechanism.md
[warning] 5-5: Blank line inside blockquote
(MD028, no-blanks-blockquote)
[warning] 179-179: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 188-188: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 191-191: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 194-194: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 200-200: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 203-203: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 206-206: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 214-214: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (14)
.gitignore (1)
26-27: LGTM!.github/PULL_REQUEST_TEMPLATE.md (1)
22-27: LGTM!AGENTS.md (1)
10-22: LGTM!Also applies to: 136-142, 224-238, 335-341, 372-409
CONTRIBUTING.md (1)
22-27: LGTM!docs/AI_DEVELOPMENT.md (1)
48-60: LGTM!Also applies to: 83-83
README.md (1)
73-89: LGTM!Also applies to: 104-109, 199-216, 254-258, 311-313
docs/README.md (1)
31-31: LGTM!bindings/rust/libdrmtap-sys/README.md (1)
36-43: LGTM!contrib/integrations/README.md (1)
10-12: LGTM!contrib/integrations/rustdesk/README.md (1)
7-11: LGTM!patches/rustdesk/README.md (1)
34-42: LGTM!SECURITY.md (1)
33-36: LGTM!Also applies to: 68-99, 111-117, 134-143, 158-161
docs/research/03_permissions.md (1)
86-87: LGTM!Also applies to: 132-139
docs/research/01_wayland_capture_problem.md (1)
29-30: LGTM!Also applies to: 54-55, 124-124
| > ⚠️ **Testing status**: Capture pipeline verified with the V3 zero-copy path on | ||
| > `virtio_gpu` (QEMU/Parallels VMs), Intel Meteor Lake (`i915`, dual 3840x2160, | ||
| > EGL CCS detiling), and NVIDIA Jetson Orin Nano (`nvidia-drm`, aarch64, Wayland). | ||
| > The AMD (`amdgpu`) backend is implemented but still awaits real hardware testing. | ||
| > If you test on real hardware, please [report results](https://github.com/fxd0h/libdrmtap/issues). | ||
|
|
||
| > ℹ️ **virgl note**: Plain `virtio-gpu` is captured with a direct linear map. A | ||
| > host-rendered **virgl** (3D) scanout cannot be read by the guest CPU (it comes | ||
| > out black, and `TRANSFER_FROM_HOST` does not bring it back); it is captured via | ||
| > GPU-side EGL readback on the guest. Solved technically; production integration is | ||
| > in progress ([#15](https://github.com/fxd0h/libdrmtap/issues/15)). | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the empty quoted lines.
The blank > lines at Lines 97 and 103 trigger MD028 and will fail markdown lint. Keep the callouts contiguous instead of inserting empty lines inside the blockquote.
♻️ Proposed fix
> If you test on real hardware, please [report results](https://github.com/fxd0h/libdrmtap/issues).
->
> ℹ️ **virgl note**: Plain `virtio-gpu` is captured with a direct linear map. A
@@
> Solved technically; production integration is
> in progress ([`#15`](https://github.com/fxd0h/libdrmtap/issues/15)).
->
> 🚧 **HDR is not ready.** HDR10 / 10-bit scanouts are **not** properly supported🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 97-97: Blank line inside blockquote
(MD028, no-blanks-blockquote)
[warning] 103-103: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 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 `@README.md` around lines 92 - 103, Remove the empty quoted lines in the README
blockquotes so the callouts stay contiguous and markdown lint does not flag
MD028. Edit the quoted sections around the testing status and virgl note text,
keeping the content directly adjacent without blank lines inside the blockquote.
Source: Linters/SAST tools
…names, install order) - CONTRIBUTING: make the real-GPU integration-test example set DRM_DEVICE explicitly (or note auto-detection), consistent with AGENTS.md. - 05_api_and_architecture: reword the privilege boundary so it is precise — DRM gates foreign-framebuffer access on master/CAP_SYS_ADMIN, the helper holds it and exports the scanout, the unprivileged library maps and reads. - 06_github_issues_analysis: use a single fourcc naming scheme for the HDR formats (AR30/XR30, XR48/AR48) instead of mixing short and long names. - 08_reframe_egl_analysis: soften the 'handles ALL GPU tiling formats' claim to the tested vendors/modifiers, and note that CCS-compressed modifiers have no CPU fallback (EGL required). - SECURITY: in the install snippet, restrict ownership/mode (0750) before the setcap grant so the binary is never world-executable while it already carries the capability.
|
Thanks, addressed all six in the latest push (e11982b):
All internal links verified (0 broken). |
- README: join the three adjacent status blockquotes with quoted-blank lines so they do not trip MD028. - 08_reframe_egl_analysis: give the dispatch diagram an explicit CCS/compressed -> -ENOTSUP branch (not a CPU fallback) and a fenced-code language.
|
Round 2:
|
1 participant
Brings the repo docs in line with what actually shipped, and removes the
.antigravity/agent-orchestration config (local tooling, not part of the project; now gitignored).Correctness fixes (claims that were no longer true)
TRANSFER_FROM_HOST+ mmap-cache mechanism with unsupported FPS figures. Replaced with the V2-copy / V3-zero-copy-DMA-BUF model and the measured per-core CPU figures. Plain virtio still usesTRANSFER_FROM_HOST(direct path) / V3 (helper path); only the virgl 3D scanout needs GPU EGL readback, captured correctly and tracked for production integration in virgl-rendered virtio-gpu scanout captures as a black frame (host-side resource, no guest readback) #15.SO_PEERCREDpeer-uid match,O_RDONLYdevice open, the default-KILL seccomp allowlist withopen/openatdeliberately excluded, exploit-mitigation flags, and the DoS geometry guard.Kept on purpose
The AI-assisted-development story (
docs/AI_DEVELOPMENT.md,AGENTS.md, the README section) is refreshed, not removed — the project documents and welcomes AI-assisted development. Versions (libdrmtap-sys 0.4.3 / libdrmtap 0.3.2), internal links, and each document's voice are preserved.Removed
.antigravity/(9 files: orchestrator/agents config + personas) — local agent-orchestration tooling that does not belong in the repo. Added to.gitignore.Internal markdown links verified (0 broken). Docs-only change; no code touched.