feat(remote): support texture/RT/depth export in remote replay mode#237
Conversation
…de (#236) Remote sessions previously rejected tex_export, rt_export and rt_depth because they call SaveTexture, which writes to the remote daemon host. In remote mode, fetch raw pixels with GetTextureData (already wired over JSON-RPC) and decode them locally with numpy + Pillow: - Add _decode_texture_png helper handling Regular formats (UNorm/SRGB 8/16-bit, Float16/32 with clip + sRGB OETF, BGRA swap, 1-4 channels) and contrast-stretched grayscale for depth targets. - Reject block-compressed/packed/MSAA formats and length mismatches with a clear error instead of producing a corrupt image. - Keep the local SaveTexture path unchanged and rt_overlay still blocked in remote mode (needs GPU compositing). - Extend mock_renderdoc with ResourceFormatType for unit coverage.
…trengthen depth test (#236)
…ote decode (#236) - decode 3D textures (depth>1) by tiling depth slices vertically instead of rejecting them; depth==1 stays byte-for-byte identical - sanitize NaN/Inf in float HDR via nan_to_num before clip (no black output, no numpy RuntimeWarning) - wrap GetTextureData, write_bytes/stat, and empty raw guard in tex_raw so remote I/O failures surface as descriptive -32002 instead of -32603
Add call_with_code() to distinguish a missing resource (-32001) from an unsupported operation (-32002). snapshot now stops the color loop only on -32001 (target absent) and skip-and-warns on -32002 (decode unsupported) instead of silently truncating the bundle. Depth export emits a stderr warning when remote decode is unsupported (e.g. D24S8/MSAA).
Add coverage for the remote-decode and snapshot fixes: 3D depth-slice tiling, NaN HDR rendering black without warning, cc=2/cc=3 channel expansion, R16G16_SNORM signed remap, B8G8R8A8_SRGB BGRA swap pixel values, and snapshot skip-and-warn behavior on -32002 color/depth decode failures.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughReplaces remote SaveTexture with controller.GetTextureData + client-side decode to produce PNGs; adds call_with_code RPC helper, format-aware decode helpers, routes tex_export/rt_export/rt_depth through remote decode, updates mocks/tests, and documents the design and test plan. ChangesRemote Texture Export via GetTextureData
Sequence Diagram(s)sequenceDiagram
participant Client
participant Daemon
participant Decoder
participant Filesystem
Client->>Daemon: GetTextureData(resourceId, subresource)
Daemon-->>Client: raw texel bytes OR JSON-RPC error(code)
Client->>Decoder: _decode_texture_png(raw, format, is_depth)
Decoder-->>Client: PNG bytes OR None (unsupported)
Client->>Filesystem: write PNG to caller path
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
openspec/changes/2026-06-09-remote-texture-export/test-plan.md (1)
105-109: ⚡ Quick winRun command is environment-specific; provide a portable default.
The command hardcodes a local interpreter path/version, which is brittle for other contributors. Consider documenting a generic command first (e.g.,
pytest ...) and keeping env-specific variants as optional examples.🤖 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 `@openspec/changes/2026-06-09-remote-texture-export/test-plan.md` around lines 105 - 109, The test-run command hardcodes an environment-specific Python path (RENDERDOC_PYTHON_PATH and ./.venv-236/bin/python); change the documentation to show a portable default first (e.g., a plain pytest invocation targeting tests/unit/test_tex_stats_handler.py) and then add the environment-specific variant as an optional example—mention RENDERDOC_PYTHON_PATH and the venv command as alternate examples rather than the primary command so contributors can run `pytest tests/unit/test_tex_stats_handler.py -q` by default.
🤖 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 `@openspec/changes/2026-06-09-remote-texture-export/proposal.md`:
- Around line 94-97: The Scope section currently lists 3D textures as "Out of
scope" but the implementation actually supports depth > 1 via slice tiling;
update the proposal text so it matches the code: either remove "3D textures
(`depth > 1`)" from the Out of scope list or change it to "In scope" and add a
short note describing the supported slice-tiling behavior (reference the
`tex_export`, `rt_export`, `rt_depth` features and the implementation's handling
of `depth > 1` / slice tiling). Ensure the wording clarifies any limitations
(e.g., max depth or tiling rules) so the spec and the decode implementation
remain consistent.
In `@openspec/changes/2026-06-09-remote-texture-export/test-plan.md`:
- Around line 105-107: The fenced code block containing the command starting
with "RENDERDOC_PYTHON_PATH=/usr/lib/python3.14/site-packages
./.venv-236/bin/python -m pytest tests/unit/test_tex_stats_handler.py -q" should
include a language tag (e.g., bash) after the opening backticks to satisfy
markdown linting and improve readability; update the fence from ``` to ```bash
so the block is recognized as bash/sh.
In `@src/rdc/handlers/_helpers.py`:
- Around line 356-360: The float RGBA export path currently applies _srgb_encode
to all channels (in the branch where ct == int(rd.CompType.Float)), which
incorrectly gamma-encodes alpha; change the logic in that branch so you apply
_srgb_encode only to the RGB channels of f (e.g., f[..., :3]) and leave the
alpha channel (f[..., 3] if present) in linear space, then combine the encoded
RGB and linear alpha, multiply by 255.0, round, and cast to uint8 to produce
rgba8; ensure you still handle arrays without alpha safely and preserve the
existing clipping/sanitization steps (sanitized, f) and use the same dtype
conversions.
In `@src/rdc/handlers/texture.py`:
- Around line 154-156: Wrap the call to controller.GetTextureData inside
_handle_tex_raw (the raw_data = controller.GetTextureData(...) line) in a
try/except; if GetTextureData raises, catch the exception and return the
JSON-RPC error using _error_response(request_id, -32002, f"GetTextureData error:
{e}") (or similar) and True so the handler returns a structured error instead of
letting the exception propagate. Ensure you reference the same request_id and
preserve the existing empty-data check path.
---
Nitpick comments:
In `@openspec/changes/2026-06-09-remote-texture-export/test-plan.md`:
- Around line 105-109: The test-run command hardcodes an environment-specific
Python path (RENDERDOC_PYTHON_PATH and ./.venv-236/bin/python); change the
documentation to show a portable default first (e.g., a plain pytest invocation
targeting tests/unit/test_tex_stats_handler.py) and then add the
environment-specific variant as an optional example—mention
RENDERDOC_PYTHON_PATH and the venv command as alternate examples rather than the
primary command so contributors can run `pytest
tests/unit/test_tex_stats_handler.py -q` by default.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46c1f031-7af2-4b9f-b170-87d59fcfbfa1
📒 Files selected for processing (10)
openspec/changes/2026-06-09-remote-texture-export/proposal.mdopenspec/changes/2026-06-09-remote-texture-export/tasks.mdopenspec/changes/2026-06-09-remote-texture-export/test-plan.mdsrc/rdc/commands/_helpers.pysrc/rdc/commands/snapshot.pysrc/rdc/handlers/_helpers.pysrc/rdc/handlers/texture.pytests/mocks/mock_renderdoc.pytests/unit/test_snapshot_command.pytests/unit/test_tex_stats_handler.py
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
…mote export (#240) * feat(remote): decode R11G11B10F and R9G9B9E5 packed HDR formats in remote export Both formats are common HDR render-target and IBL storage formats that PR #237 scoped out. They are closed-form bit-unpackable in numpy with no GPU round-trip, so they follow the existing local-decode path for Regular Float formats. Add _unpack_r11g11b10 and _unpack_r9g9b9e5 vectorised helpers and branch on them in _decode_texture_png before the Regular gate, carrying their own MSAA reject and local width/height/depth/length checks. Decoded RGB feeds the existing Float display pipeline (nan_to_num, clip, sRGB encode, opaque alpha, depth tiling). Repurpose the former R11G11B10 rejection test to R5G6B5, which remains an unsupported packed format after this change. * docs(spec): correct subnormal decode value annotation
Closes #236.
Problem
rdc texture,rdc rt, and depth export returned "not supported in remote mode" when connected to a remote replayserver (--proxy/ split / android), blocking CI pipelines that replay on a remote GPU target. Root cause: these handlers usedSaveTexture(), which writes to the remote replayserver's filesystem, unreachable by the local daemon.Solution
In remote mode, use
controller.GetTextureData()(raw pixels over the JSON-RPC wire) and decode locally with numpy + Pillow. Local mode is unchanged (stillSaveTexture)._decode_dtype/_decode_texture_png: handles Float16/32, UNorm8/16, UNormSRGB8, SNorm8/16 (signed-remapped), UInt8/16, Depth16/32, BGRA channel swap, 1–4 components, 3D textures (depth slices tiled), NaN-safe HDR clamp + sRGB OETF.-32002error instead of a corrupt image.rt_overlaystays remote-blocked (needs GPU compositing).rdc snapshotnow surfaces skipped targets in remote mode instead of silently dropping them.Testing
renderdoccmd remoteserver+--proxy) on a vkcube capture:textureandrtexports are pixel-identical to localSaveTextureoutput; D16 depth renders a correct grayscale gradient. Verified end-to-end through the normalrdc open --proxyCLI path.Known limitation
Depth visualization differs between local (RenderDoc renders depth into the red channel via
SaveTexture) and remote (grayscale min-max stretch). Tracked for a follow-up to unify.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation