Fixes terrain origin computing in the inverted pyramid slope terrain

[luoye2333]

Description

In height_field_to_mesh function, the terrain origin is computed as the max in the square area around the center of the terrain,
and the square has a fixed width of 2 meters.

        # compute origin
        x1 = int((cfg.size[0] * 0.5 - 1) / cfg.horizontal_scale)  # fixed width
        x2 = int((cfg.size[0] * 0.5 + 1) / cfg.horizontal_scale)
        y1 = int((cfg.size[1] * 0.5 - 1) / cfg.horizontal_scale)
        y2 = int((cfg.size[1] * 0.5 + 1) / cfg.horizontal_scale)
        origin_z = np.max(heights[x1:x2, y1:y2]) * cfg.vertical_scale
        origin = np.array([0.5 * cfg.size[0], 0.5 * cfg.size[1], origin_z])

In inverted pyramid slope terrain, the default platform_width is 1.0 meter. Therefore the area outside the platform is also calculated in the np.max function. This caused the terrain origin to be a little higher than the ground. See the Screenshots below. Non-inverted pyramid terrain do not have this problem because the function is max.

To fix, a param terrain_origin_judge_width is added to the height_field_to_mesh wrapper (default is 2.0 so the behaviour is same as before) and we can use decorator @height_field_to_mesh_v2(terrain_origin_judge_width=1.0) to change the max function area. For the default platform 1.0 meter in inverted pyramid slope terrain, a half width terrain_origin_judge_width=1.0 is ok.

        # compute origin
        x1 = int((cfg.size[0] * 0.5 - terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
        x2 = int((cfg.size[0] * 0.5 + terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
        y1 = int((cfg.size[1] * 0.5 - terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
        y2 = int((cfg.size[1] * 0.5 + terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)

Type of change

• Bug fix (non-breaking change which fixes an issue)

Screenshots

Please attach before and after screenshots of the change if applicable.

Before	After

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[greptile-apps]

Greptile Overview

Greptile Summary

Fixes terrain origin calculation for inverted pyramid slope terrain by introducing a configurable terrain_origin_judge_width parameter.

Key Changes:

• Added new decorator height_field_to_mesh_v2 that accepts terrain_origin_judge_width parameter (defaults to 2.0m for backward compatibility)
• Modified pyramid_sloped_terrain to use the new decorator with terrain_origin_judge_width=1.0 to match the default platform width
• This prevents the origin calculation from sampling outside the 1.0m platform area in inverted pyramids, which was causing the origin to be incorrectly positioned above the ground

Issues Found:

• Missing input validation for terrain_origin_judge_width - could cause index out of bounds or empty slice errors if the parameter is larger than terrain size or causes invalid slice indices
• Docstring for height_field_to_mesh_v2 doesn't follow the same format/style as the original height_field_to_mesh decorator
• Code duplication between height_field_to_mesh and height_field_to_mesh_v2 (53 lines of identical logic)

Confidence Score: 3/5

• This PR is functionally correct but has input validation gaps that could cause runtime errors in edge cases
• The core fix correctly addresses the inverted pyramid terrain origin issue by allowing configurable sampling width. However, missing validation for the terrain_origin_judge_width parameter could lead to index out of bounds errors or empty slice failures when the parameter is misconfigured. The logic errors identified need to be addressed before merge.
• Pay close attention to source/isaaclab/isaaclab/terrains/height_field/utils.py - needs validation logic added

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/terrains/height_field/utils.py	Added height_field_to_mesh_v2 decorator with configurable terrain origin computation width parameter
source/isaaclab/isaaclab/terrains/height_field/hf_terrains.py	Changed pyramid_sloped_terrain decorator to use height_field_to_mesh_v2 with 1.0m origin judge width

Sequence Diagram

sequenceDiagram
    participant User
    participant pyramid_sloped_terrain
    participant height_field_to_mesh_v2
    participant convert_height_field_to_mesh
    
    User->>pyramid_sloped_terrain: Call with difficulty, cfg
    Note over pyramid_sloped_terrain: Decorated with @height_field_to_mesh_v2(terrain_origin_judge_width=1.0)
    pyramid_sloped_terrain->>height_field_to_mesh_v2: Execute decorator wrapper
    height_field_to_mesh_v2->>height_field_to_mesh_v2: Validate border_width
    height_field_to_mesh_v2->>height_field_to_mesh_v2: Allocate height field buffer with border
    height_field_to_mesh_v2->>pyramid_sloped_terrain: Call original function with adjusted cfg
    pyramid_sloped_terrain-->>height_field_to_mesh_v2: Return height field array (z_gen)
    height_field_to_mesh_v2->>convert_height_field_to_mesh: Convert heights to mesh
    convert_height_field_to_mesh-->>height_field_to_mesh_v2: Return vertices, triangles
    height_field_to_mesh_v2->>height_field_to_mesh_v2: Compute origin using terrain_origin_judge_width=1.0
    Note over height_field_to_mesh_v2: Uses 1.0m square area instead of default 2.0m<br/>to avoid sampling outside platform in inverted pyramid
    height_field_to_mesh_v2-->>User: Return [mesh], origin

Loading

[greptile-apps]

_{3 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

docstring doesn't follow the same format as height_field_to_mesh. Should include Args, Returns sections and proper capitalization/punctuation

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

[greptile-apps]

potential index out of bounds if terrain_origin_judge_width is larger than the terrain size. Consider adding validation

Suggested change

	x1 = int((cfg.size[0] * 0.5 - terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
	x2 = int((cfg.size[0] * 0.5 + terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
	y1 = int((cfg.size[1] * 0.5 - terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
	y2 = int((cfg.size[1] * 0.5 + terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
	origin_z = np.max(heights[x1:x2, y1:y2]) * cfg.vertical_scale
	# compute origin
	if terrain_origin_judge_width > min(cfg.size[0], cfg.size[1]):
	raise ValueError(
	f"The terrain origin judge width ({terrain_origin_judge_width}) must be smaller than "
	f"the minimum terrain size ({min(cfg.size[0], cfg.size[1])})."
	)
	x1 = int((cfg.size[0] * 0.5 - terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
	x2 = int((cfg.size[0] * 0.5 + terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
	y1 = int((cfg.size[1] * 0.5 - terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)
	y2 = int((cfg.size[1] * 0.5 + terrain_origin_judge_width / 2.0) / cfg.horizontal_scale)

[greptile-apps]

if x1 >= x2 or y1 >= y2 due to rounding, np.max(heights[x1:x2, y1:y2]) will fail. Check that indices form valid slices

[Mayankm96]

Hi, thank you for sharing the bug and potential fix.

This parameter terrain_origin_judge_width seems to depend a lot on the choice of the platform width size. I don't think hard-coding it here like this is the right solution.

The proper solution should be that the height-field generator also returns the origin. Then all this hard-coding logic inside height_field_to_mesh can be avoided.