Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/rdc/handlers/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,14 +235,24 @@ def _walk(actions: list[Any]) -> None:
populate_shaders_subtree(state.vfs_tree, state.shader_meta)


def _make_texsave(rd: Any, resource_id: Any, mip: int = 0, array_slice: int = 0) -> Any:
"""Create a TextureSave object using the renderdoc module."""
def _make_texsave(
rd: Any,
resource_id: Any,
mip: int = 0,
array_slice: int = 0,
dest_type: Any | None = None,
) -> Any:
"""Create a TextureSave object using the renderdoc module.

PNG remains the command-level default. ``dest_type`` is retained here so
future CLI format selection reaches RenderDoc's own exporter unchanged.
"""
ts = rd.TextureSave()
ts.resourceId = resource_id
ts.mip = mip
ts.slice = rd.TextureSliceMapping()
ts.slice.sliceIndex = array_slice
ts.destType = rd.FileType.PNG
ts.destType = rd.FileType.PNG if dest_type is None else dest_type
return ts


Expand Down
80 changes: 53 additions & 27 deletions src/rdc/handlers/texture.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,6 @@
}


def _is_block_compressed(fmt: Any) -> bool:
"""Return whether RenderDoc identifies a format as block-compressed."""
block_format = getattr(fmt, "BlockFormat", None)
renderdoc_result = bool(block_format()) if callable(block_format) else False
return renderdoc_result or fmt.Name().upper().startswith(("ASTC", "BC", "EAC", "ETC", "PVRTC"))


def _select_3d_slice(tex: Any, raw: bytes, mip: int, array_slice: int, rd: Any) -> bytes:
"""Return one depth slice when remote GetTextureData returns a whole 3D mip."""
depth = max(1, tex.depth >> mip)
Expand Down Expand Up @@ -90,7 +83,7 @@ def _handle_tex_info(
), True


def _export_remote(
def _export_decoded(
request_id: int,
state: DaemonState,
tex: Any,
Expand All @@ -100,9 +93,8 @@ def _export_remote(
array_slice: int = 0,
*,
is_depth: bool = False,
allow_save_fallback: bool = False,
) -> tuple[dict[str, Any], bool]:
"""Fetch raw pixels over the wire and decode them locally to a PNG."""
"""Fetch raw pixels and decode them locally for the local depth-export path."""
controller = state.adapter.controller # type: ignore[union-attr]
sub = _make_subresource(state.rd, mip, array_slice)
try:
Expand All @@ -118,18 +110,9 @@ def _export_remote(
state.rd, tex, raw, mip, is_depth=is_depth, depth_override=depth_override
)
if png is None:
if allow_save_fallback and _is_block_compressed(tex.format):
texsave = _make_texsave(state.rd, resource_id, mip, array_slice)
success = controller.SaveTexture(texsave, str(temp_path))
if success and temp_path.exists():
return _result_response(
request_id,
{"path": str(temp_path), "size": temp_path.stat().st_size},
), True
return _error_response(request_id, -32002, "SaveTexture fallback failed"), True
fmt_name = tex.format.Name() if hasattr(tex.format, "Name") else ""
return _error_response(
request_id, -32002, f"format {fmt_name} not supported for remote decode"
request_id, -32002, f"format {fmt_name} not supported for local depth decode"
), True
try:
temp_path.write_bytes(png)
Expand All @@ -139,6 +122,52 @@ def _export_remote(
return _result_response(request_id, {"path": str(temp_path), "size": size}), True


def _save_result_ok(result: Any) -> bool:
"""Accept current ResultDetails and older bool SaveTexture bindings."""
ok = getattr(result, "OK", None)
return bool(ok()) if callable(ok) else bool(result)
Comment on lines +125 to +128

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

RenderDoc python ReplayController SaveTexture return type ResultDetails

💡 Result:

The SaveTexture method of the RenderDoc ReplayController class does not return a ResultDetails object; its return type is None [1]. The method is defined as: controller.SaveTexture(saveData, path) Where: - saveData is a renderdoc.TextureSave object that specifies the configuration for saving the texture (e.g., resource ID, file type, mapping options) [2][3]. - path is a string representing the destination file path on disk [1]. While other methods in the RenderDoc API, such as CaptureFile.OpenCapture, return a tuple containing a ResultDetails object (which is used to handle success or error reporting) [4][5], SaveTexture performs the operation and does not provide an explicit result object as a return value [1].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the file structure first
ast-grep outline src/rdc/handlers/texture.py --view expanded

# Read the relevant section with line numbers
sed -n '180,340p' src/rdc/handlers/texture.py | cat -n

Repository: BANANASJIM/rdc-cli

Length of output: 8902


🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/rdc/handlers/texture.py --view expanded
printf '\n---\n'
sed -n '180,340p' src/rdc/handlers/texture.py | cat -n

Repository: BANANASJIM/rdc-cli

Length of output: 8907


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the specific handler functions and any save result helpers
rg -n "_save_result_ok|_save_result_message|SaveTexture|temp_path.exists|if not success" src/rdc/handlers/texture.py

printf '\n--- context ---\n'
sed -n '180,340p' src/rdc/handlers/texture.py

Repository: BANANASJIM/rdc-cli

Length of output: 8999


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '120,170p' src/rdc/handlers/texture.py | cat -n

Repository: BANANASJIM/rdc-cli

Length of output: 2466


Local SaveTexture paths always trip the failure check. In _handle_tex_export, _handle_rt_export, and the depth fallback in _handle_rt_depth, controller.SaveTexture(...) returns None in these bindings, so if not success or not temp_path.exists() reports SaveTexture failed even when the file was written. Drop the success truthiness check and rely on the file write/exception path.

🤖 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 `@src/rdc/handlers/texture.py` around lines 125 - 128, Update
_handle_tex_export, _handle_rt_export, and the depth fallback in
_handle_rt_depth to stop treating the return value of
controller.SaveTexture(...) as a success flag. Rely on exceptions and
temp_path.exists() to detect write failure, while preserving the existing
failure handling and the _save_result_ok helper for paths where its result is
meaningful.



def _save_result_message(result: Any) -> str:
"""Return a RenderDoc failure message when the binding provides one."""
message = getattr(result, "Message", None)
return str(message()) if callable(message) else "SaveTexture failed"


def _export_remote(
request_id: int,
state: DaemonState,
resource_id: Any,
temp_path: Path,
mip: int,
array_slice: int = 0,
*,
dest_type: Any | None = None,
) -> tuple[dict[str, Any], bool]:
"""Export through RenderDoc's local SaveTexture implementation.

In remote replay, the controller lives locally and fetches image data via
the replay proxy. This is the same path used by qrenderdoc and preserves
RenderDoc's handling of compressed, HDR, 3D, and special formats.
"""
controller = state.adapter.controller # type: ignore[union-attr]
texsave = _make_texsave(state.rd, resource_id, mip, array_slice, dest_type)
try:
result = controller.SaveTexture(texsave, str(temp_path))
except Exception as exc: # noqa: BLE001
return _error_response(request_id, -32002, f"SaveTexture failed: {exc}"), True
if not _save_result_ok(result):
return _error_response(request_id, -32002, _save_result_message(result)), True
if not temp_path.exists():
return _error_response(
request_id, -32002, "SaveTexture did not create an output file"
), True
return _result_response(
request_id,
{"path": str(temp_path), "size": temp_path.stat().st_size},
), True


def _handle_tex_export(
request_id: int, params: dict[str, Any], state: DaemonState
) -> tuple[dict[str, Any], bool]:
Expand Down Expand Up @@ -174,12 +203,10 @@ def _handle_tex_export(
return _export_remote(
request_id,
state,
tex,
tex.resourceId,
temp_path,
mip,
array_slice,
allow_save_fallback=True,
)
controller = state.adapter.controller
texsave = _make_texsave(state.rd, tex.resourceId, mip, array_slice)
Expand Down Expand Up @@ -247,10 +274,7 @@ def _handle_rt_export(
resource = match[0].resource
temp_path = state.temp_dir / f"rt_{eid}_color{target_idx}.png"
if state.is_remote:
tex = state.tex_map.get(int(resource))
if tex is None:
return _error_response(request_id, -32001, f"target {int(resource)} not found"), True
return _export_remote(request_id, state, tex, resource, temp_path, 0)
return _export_remote(request_id, state, resource, temp_path, 0)
texsave = _make_texsave(state.rd, resource)
success = state.adapter.controller.SaveTexture(texsave, str(temp_path)) # type: ignore[union-attr]
if not success or not temp_path.exists():
Expand Down Expand Up @@ -281,7 +305,9 @@ def _handle_rt_depth(
return _error_response(
request_id, -32001, f"depth target {int(depth.resource)} not found"
), True
resp, running = _export_remote(
if state.is_remote:
return _export_remote(request_id, state, depth.resource, temp_path, 0)
resp, running = _export_decoded(
request_id, state, tex, depth.resource, temp_path, 0, is_depth=True
)
# Combined depth-stencil and MSAA formats decode to None (-32002). Locally,
Expand Down
Loading
Loading