Fix Newton marker world rendering

[ooctipus]

Summary

• add NewtonVisualizerCfg.world_spacing for visual-only separation of co-located simulation worlds
• render dense environment-major marker batches using the same selected worlds and offsets as model geometry
• select marker state along its environment axis and broadcast offsets without flattened gather or repeated-offset tensors
• cache marker prototype conversion and keep filtering/offset application in one typed render path
• resolve and deduplicate visible environment IDs once, then pass the exact selection to the Newton viewer
• document partial Newton visualization and its dense environment-major marker convention

Non-divisible marker batches remain global and are neither filtered nor offset.

Testing

• 22 passed: source/isaaclab_visualizers/test/test_newton_adapter.py
• 10 passed: scoped Newton marker render and partial-update tests
• CPU marker state with CUDA viewer offsets: selected worlds produced [12, 13, 36, 37]
• all-world offset smoke: [0, 1, 12, 13, 24, 25, 36, 37]
• non-divisible global marker smoke remained [0, 1, 2]
• fail-before-fix verification: old filtering produced [1, 3, 5, 7] instead of [12, 13, 36, 37]; the old allocation path also invoked the forbidden torch.arange regression guard
• dynamic reordered selections [3, 1] and [1, 3] preserve transform, scale, and source tensor state
• ./isaaclab.sh -d passed with warnings treated as errors
• scoped pre-commit passed, including the LFS hook
• repository-wide pre-commit passed with only the known unrelated golden-image LFS hook skipped
• changelog fragment check passed

[greptile-apps]

Greptile Summary

This PR fixes environment-major ordering in Newton marker filtering (previously the loop was world-major, producing wrong instance indices), adds per-world visual offset application to marker overlays, exposes world_spacing in NewtonVisualizerCfg, and promotes marker rendering to a public render_markers method for use by other Newton viewers.

• Core bug fix: _filter_marker_state was iterating (block_idx × num_envs + env_id) instead of (env_id × repeat_count + i), causing markers to be pulled from wrong positions in environment-major arrays; the fix correctly slices contiguous per-env blocks.
• World offset application: _apply_marker_world_offsets now looks up each marker's owning world from a new world_indices field and shifts translations by the viewer's world_offsets, with duck-typed fallbacks for torch, numpy, and warp tensor types.
• Public API addition: render_markers(viewer, num_envs) is extracted from the internal render loop so other Newton-family viewers can consume the same marker overlays.

Confidence Score: 4/5

Safe to merge; the core filter rewrite is correct and well-tested, and the two concerns are edge cases in the new public API surface rather than bugs in the primary render path.

The env-major slicing fix is the right approach and all three new test cases verify the expected translation values end-to-end. The two flagged items are both in _apply_marker_world_offsets: one is a private-attribute coupling that has no impact unless a viewer has an unrelated _user_spacing zero-value, and the other is a missing bounds guard that would only trigger if render_markers is called with a viewer initialised for fewer worlds than num_envs. Neither affects the existing NewtonVisualizer call-site.

source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualization_markers.py — specifically _apply_marker_world_offsets around the early-exit guard and the index_select call.

Important Files Changed

Filename	Overview
source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualization_markers.py	Core logic change: environment-major filter rewrite, new world_indices tracking, _apply_marker_world_offsets; contains a minor bounds-check gap on index_select and a private-attribute coupling in the early-exit guard.
source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py	Extracted render_markers as a public method and wired world_spacing into set_world_offsets; straightforward refactor with no functional issues.
source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer_cfg.py	Adds world_spacing tuple field with sensible (0,0,0) default and clear docstring; no issues.
source/isaaclab/test/markers/test_visualization_markers.py	Updates expected filter result to env-major indices and adds two regression tests for world-offset application; test values verified against the new implementation.
source/isaaclab_visualizers/test/test_newton_adapter.py	Extends cfg test for world_spacing and adds a focused delegate test for render_markers; coverage is accurate.
source/isaaclab_visualizers/changelog.d/newton-world-spacing.minor.rst	New changelog fragment documenting both the added features and the filter fix; accurate and well-scoped.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant NewtonVisualizer
    participant render_newton_visualization_markers
    participant NewtonVisualizationMarkers
    participant _filter_marker_state
    participant _apply_marker_world_offsets
    participant ViewerGL

    Caller->>NewtonVisualizer: render_markers(viewer, num_envs)
    Note over NewtonVisualizer: guards: enable_markers, resolves num_envs
    NewtonVisualizer->>render_newton_visualization_markers: (viewer, resolved_env_ids, num_envs)
    loop each registered marker group
        render_newton_visualization_markers->>NewtonVisualizationMarkers: marker.render(viewer, env_ids, num_envs)
        NewtonVisualizationMarkers->>_filter_marker_state: (marker, env_ids, num_envs)
        Note over _filter_marker_state: env-major slicing: env_id x repeat_count builds world_indices tensor
        _filter_marker_state-->>NewtonVisualizationMarkers: state with translations and world_indices
        NewtonVisualizationMarkers->>_apply_marker_world_offsets: (viewer, translations, world_indices, num_envs)
        Note over _apply_marker_world_offsets: reads viewer.world_offsets, index_select by world_indices
        _apply_marker_world_offsets-->>NewtonVisualizationMarkers: offset translations
        NewtonVisualizationMarkers->>ViewerGL: log_instances / log_lines
    end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant NewtonVisualizer
    participant render_newton_visualization_markers
    participant NewtonVisualizationMarkers
    participant _filter_marker_state
    participant _apply_marker_world_offsets
    participant ViewerGL

    Caller->>NewtonVisualizer: render_markers(viewer, num_envs)
    Note over NewtonVisualizer: guards: enable_markers, resolves num_envs
    NewtonVisualizer->>render_newton_visualization_markers: (viewer, resolved_env_ids, num_envs)
    loop each registered marker group
        render_newton_visualization_markers->>NewtonVisualizationMarkers: marker.render(viewer, env_ids, num_envs)
        NewtonVisualizationMarkers->>_filter_marker_state: (marker, env_ids, num_envs)
        Note over _filter_marker_state: env-major slicing: env_id x repeat_count builds world_indices tensor
        _filter_marker_state-->>NewtonVisualizationMarkers: state with translations and world_indices
        NewtonVisualizationMarkers->>_apply_marker_world_offsets: (viewer, translations, world_indices, num_envs)
        Note over _apply_marker_world_offsets: reads viewer.world_offsets, index_select by world_indices
        _apply_marker_world_offsets-->>NewtonVisualizationMarkers: offset translations
        NewtonVisualizationMarkers->>ViewerGL: log_instances / log_lines
    end

Loading

_{Reviews (1): Last reviewed commit: "Fix Newton marker world rendering" | Re-trigger Greptile}

[greptile-apps]

The _user_spacing early-exit guard reads a private attribute (_user_spacing) that only exists on NewtonViewerGL. When render_markers is called with an arbitrary external viewer this attribute won't be present and getattr silently returns None, which is fine — but the inverse is also true: any viewer that happens to carry _user_spacing = (0, 0, 0) for unrelated reasons will silently suppress offset application even when world_offsets is non-zero. The safe check is the presence of world_offsets itself, which is already tested two lines later.

Suggested change

	spacing = getattr(viewer, "_user_spacing", None)
	if spacing is not None and all(float(spacing[axis]) == 0.0 for axis in range(3)):
	return translations
	world_offsets = getattr(viewer, "world_offsets", None)
	world_offsets = getattr(viewer, "world_offsets", None)

[greptile-apps]

Missing bounds guard before index_select

world_indices values are env-ids in [0, num_envs), but offsets.shape[0] equals however many worlds the passed viewer was initialised for. Via the new public render_markers(other_viewer, num_envs) API it's easy to pass a viewer whose world_offsets was built for a different (smaller) world count, causing index_select to throw an out-of-range error at runtime. Adding a guard like if offsets.shape[0] < num_envs: return translations before this block would prevent the crash and match the defensive style used elsewhere in the function.