feat(scene-engine): add text-guided scene editing - #515
Open
MuziWong wants to merge 24 commits into
Open
Conversation
…mentation client)
…cs and tests may not be updated
…ption, and add check code
…ption in objects' names
…but no real-world scale...
Greptile SummaryThe PR introduces a text-guided workflow for adding, moving, and deleting Scene Engine objects while preserving importable and re-exportable scene metadata.
Confidence Score: 2/5The PR is not yet safe to merge because unchecked generated object identifiers can escape the output directory and planar update batches can silently lose requested constraints. VLM-derived categories still reach image and mask output paths without containment validation, while per-update incident-relation clearing can remove an earlier relation from the same planar update batch. Files Needing Attention: embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_understanding.py, embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_asset_preparation.py, and embodichain/gen_sim/scene_engine/core/scene_graph.py
|
| Filename | Overview |
|---|---|
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_understanding.py | Converts text instructions into validated edit operations and applies them to scene-graph updates. |
| embodichain/gen_sim/scene_engine/core/scene_graph.py | Introduces support hierarchy, tabletop-region, planar-relation, normalization, and batch-update behavior. |
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_asset_preparation.py | Generates, segments, reconstructs, and prepares assets introduced by add operations. |
| embodichain/gen_sim/scene_engine/core/scene_edit_plan.py | Defines and validates normalized add, move, and delete operations against an immutable input scene. |
| embodichain/gen_sim/scene_engine/pipeline/edit.py | Orchestrates import, understanding, asset preparation, layout generation, and re-export for scene edits. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CLI input] --> B{Image provided?}
B -->|Yes| C[Generate scene]
B -->|No| D[Load existing export]
C --> E{Edit prompt provided?}
D --> F[Interpret edit prompt]
E -->|Yes| F
E -->|No| G[Export generated scene]
F --> H[Validate SceneEditPlan]
H --> I[Update SceneGraph]
I --> J[Prepare added assets]
J --> K[Construct and optimize layout]
K --> L[Re-export scene]
Reviews (4): Last reviewed commit: "replace jiange's pca heuristic method wi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+24
to
+29
| from embodichain.gen_sim.scene_engine.configs.environment import ( | ||
| read_scene_engine_env_values, | ||
| ) | ||
|
|
||
|
|
||
| class ImageGenerationClient: |
There was a problem hiding this comment.
This new public module does not define __all__, leaving its intended API ambiguous and allowing wildcard imports to expose incidental imported names.
Suggested change
| from embodichain.gen_sim.scene_engine.configs.environment import ( | |
| read_scene_engine_env_values, | |
| ) | |
| class ImageGenerationClient: | |
| from embodichain.gen_sim.scene_engine.configs.environment import ( | |
| read_scene_engine_env_values, | |
| ) | |
| __all__ = ["ImageGenerationClient"] | |
| class ImageGenerationClient: |
Context Used: CLAUDE.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/gen_sim/scene_engine/clients/image_generation.py
Line: 24-29
Comment:
**Public client lacks export declaration**
This new public module does not define `__all__`, leaving its intended API ambiguous and allowing wildcard imports to expose incidental imported names.
```suggestion
from embodichain.gen_sim.scene_engine.configs.environment import (
read_scene_engine_env_values,
)
__all__ = ["ImageGenerationClient"]
class ImageGenerationClient:
```
**Context Used:** CLAUDE.md ([source](https://github.com/dexforce/embodichain/blob/main/CLAUDE.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.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!
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a text-guided scene editing workflow to the Scene Engine pipeline, extending exported artifacts to include an explicit SceneGraph representation and edit planning so scenes can be generated, edited, and re-exported in-place.
Changes:
- Introduces edit-time core structures (
SceneGraph,SceneEditPlan) and editing pipeline stages (understanding → asset prep → layout optimization → export). - Adds an image-generation client and updates segmentation config semantics, plus CLI support for
--edit_prompt(edit-only, generate-only, or generate-then-edit). - Persists additional spatial metadata (table support contour + conservative optimization rectangle, object
center_xy) into exports and import flows; updates docs and expands test coverage.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/gen_sim/scene_engine/test_simready_processor_utils.py | Adds regression coverage for VLM-footprint-driven uniform scaling. |
| tests/gen_sim/scene_engine/test_scene_understanding.py | Updates imports and adds validation tests for location words in descriptions + initial scene graph initialization. |
| tests/gen_sim/scene_engine/test_scene_layout_optimizer.py | Adds tests for table 9-grid mapping and stacked parent/child placement. |
| tests/gen_sim/scene_engine/test_scene_graph.py | Adds validation/normalization tests for graph layers, planar relations, and serialization. |
| tests/gen_sim/scene_engine/test_scene_engine_config.py | Adds CLI tests for edit-only and generate-then-edit behavior. |
| tests/gen_sim/scene_engine/test_scene_edit.py | Adds tests for importing editable exports and validating edit prerequisites. |
| tests/gen_sim/scene_engine/test_scene_edit_plan.py | Adds comprehensive tests for edit operation parsing, validation, graph updates, and asset prep. |
| tests/gen_sim/scene_engine/test_scene_core_and_export.py | Extends export tests to cover scene_graph.json, scene.json, center_xy, and import round-trips. |
| tests/gen_sim/scene_engine/test_clients.py | Adds image-generation client tests and updates segmentation endpoint/config keys. |
| embodichain/gen_sim/scene_engine/pipeline/utils/table_support_surface.py | Persists a conservative optimization rectangle derived from the detected tabletop contour. |
| embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py | Reworks SimReady processing to optionally query a VLM for rotation/scale and to persist support geometry. |
| embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor_utils.py | Adds rendering + VLM query helpers and GLB rotation/scale utilities for VLM-based transforms. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_layout_optimizer.py | Adds numerical optimization utilities for graph-constrained tabletop/stacked layouts. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_layout_constructor.py | Adds orchestration for applying layout optimization across table-root and stacked groups. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_importer.py | Adds importer for existing Scene Engine exports (scene + graph) to enable edit-only flows. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_exporter.py | Extends export to write scene_graph.json + scene.json and cleans up stale mesh assets. |
| embodichain/gen_sim/scene_engine/pipeline/utils/image_segmentation_utils.py | Adds heuristic mask inversion for generated single-object images and related utilities. |
| embodichain/gen_sim/scene_engine/pipeline/generation/scene_understanding.py | Returns (Scene, SceneGraph) and tightens semantic constraints to exclude location words in descriptions. |
| embodichain/gen_sim/scene_engine/pipeline/generation/scene_generation.py | Wires graph validation through generation/refinement and reuses persisted support geometry for refinement. |
| embodichain/gen_sim/scene_engine/pipeline/generation/init.py | Initializes generation subpackage. |
| embodichain/gen_sim/scene_engine/pipeline/generate.py | Updates generation entrypoint to pass segmentation client and export the scene graph. |
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_understanding.py | Adds VLM-to-edit-plan parsing and graph update application utilities. |
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_layout_generation.py | Adds layout optimization stage for applying edit plan results. |
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_asset_preparation.py | Adds image→mask→geometry→SimReady asset generation for add operations. |
| embodichain/gen_sim/scene_engine/pipeline/editing/init.py | Initializes editing subpackage. |
| embodichain/gen_sim/scene_engine/pipeline/edit.py | Adds edit pipeline entrypoint and re-export of edited outputs. |
| embodichain/gen_sim/scene_engine/core/scene_object.py | Extends SceneObject with center_xy and table support geometry fields for edits/layout. |
| embodichain/gen_sim/scene_engine/core/scene_graph.py | Adds SceneGraph data model with validation, normalization, and update application. |
| embodichain/gen_sim/scene_engine/core/scene_edit_plan.py | Adds validated edit operation and plan data structures. |
| embodichain/gen_sim/scene_engine/clients/image_segmentation.py | Renames segmentation endpoint config to “by prompt” and updates request path usage. |
| embodichain/gen_sim/scene_engine/clients/image_generation.py | Adds image-generation service client and dotenv configuration loader. |
| embodichain/gen_sim/scene_engine/cli/start.py | Adds --edit_prompt and supports edit-only / generate-then-edit invocation. |
| docs/source/features/generative_sim/scene_engine.md | Documents edit workflow, new dotenv keys, and new export artifacts. |
Suppressed comments (1)
embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py:251
_vlm_transform_for_objectwrites debug artifacts undersimready_geometry_root.parent / "debug"even when the caller provideddebug_output_root. This makes outputs harder to control/relocate and can create unexpected files during batch runs.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+385
to
+406
| best_rectangle: Polygon | None = None | ||
| best_area = 0.0 | ||
| for x_index, minimum_x in enumerate(x_values[:-1]): | ||
| for maximum_x in x_values[x_index + 1 :]: | ||
| if maximum_x <= minimum_x: | ||
| continue | ||
| for y_index, minimum_y in enumerate(y_values[:-1]): | ||
| for maximum_y in y_values[y_index + 1 :]: | ||
| if maximum_y <= minimum_y: | ||
| continue | ||
| rectangle = Polygon( | ||
| [ | ||
| (minimum_x, minimum_y), | ||
| (maximum_x, minimum_y), | ||
| (maximum_x, maximum_y), | ||
| (minimum_x, maximum_y), | ||
| ] | ||
| ) | ||
| area = rectangle.area | ||
| if area > best_area and polygon.covers(rectangle): | ||
| best_rectangle = rectangle | ||
| best_area = area |
…tate for assets who need to be calibrated
…libration in image-conditioned scene engine pipeline (but only calibrated the upright bottle-like assets currently)
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.
Suppressed comments (5)
embodichain/gen_sim/scene_engine/pipeline/utils/image_segmentation_utils.py:340
- Mask files are opened without being closed (
Image.open(mask_path)); since the convertedImageobjects are retained indecoded_masks, this can keep file handles open longer than intended. Open masks with a context manager and.copy()the converted image before storing it.
mask = Image.open(mask_path).convert("L")
_require_image_size(mask, image.size)
decoded_masks.append((asset_id, mask))
embodichain/gen_sim/scene_engine/pipeline/utils/scene_layout_constructor.py:368
- Using
list.pop(0)for the BFS queue makes group construction O(n^2) in the number of nodes. Switchingpendingto acollections.dequekeeps this O(n).
pending = [TABLE_OBJECT_ID]
while pending:
parent_id = pending.pop(0)
embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py:247
- This prompt string has a grammatical error ("needs to be place" → "needs to be placed"), which can reduce clarity for VLM prompting and for future maintainers reading the code.
embodichain/gen_sim/scene_engine/pipeline/utils/image_segmentation_utils.py:327 Image.open(...)is used without a context manager here; the underlying file handle may remain open until the image object is GC'd, which can leak descriptors in long-running pipelines. Usewith Image.open(...)and.copy()after.convert()to fully detach from the file.
This issue also appears on line 338 of the same file.
image = Image.open(image_path).convert("RGBA")
overlay = Image.new("RGBA", image.size, (0, 0, 0, 0))
embodichain/gen_sim/scene_engine/pipeline/utils/scene_layout_constructor.py:215
- Using
list.pop(0)in a loop is O(n) per iteration and can make descendant propagation quadratic as the number of nodes grows. Using acollections.dequemakes the BFS queue operations O(1).
This issue also appears on line 366 of the same file.
pending = list(children_by_parent.get(root_id, []))
while pending:
descendant_id = pending.pop(0)
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.
Suppressed comments (4)
embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py:247
- The
needed_layoutprompt string has grammatical issues ("needs to be place") and theuse_scale/use_rotationparameters are immediately deleted, which obscures intent.
embodichain/gen_sim/scene_engine/pipeline/utils/table_support_surface.py:405 _largest_inscribed_rectangledoes up to ~5Mpolygon.covers(rectangle)checks (48^4 worst-case), and each covers test can be expensive. Preparing the polygon once can significantly reduce the per-rectangle predicate cost.
area = rectangle.area
if area > best_area and polygon.covers(rectangle):
best_rectangle = rectangle
embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py:215
- VLM transform flags are effectively ignored here:
rotate_about_xand the baked GLB rotation should be conditioned onuse_vlm_rotation, and the code currently hard-fails when Blenderbpyis unavailable (common in non-Blender runtimes), which breaks scene-edit add flows.
embodichain/gen_sim/scene_engine/pipeline/utils/image_segmentation_utils.py:339 render_asset_mask_id_overlayopens images/masks without a context manager. On some platforms this can leave file descriptors open longer than needed (and can prevent deleting temp directories on Windows).
image = Image.open(image_path).convert("RGBA")
overlay = Image.new("RGBA", image.size, (0, 0, 0, 0))
colors = (
(239, 83, 80, 255),
(66, 165, 245, 255),
(102, 187, 106, 255),
(255, 202, 40, 255),
(171, 71, 188, 255),
(38, 198, 218, 255),
)
decoded_masks: list[tuple[str, Image.Image]] = []
for index, (asset_id, mask_path) in enumerate(asset_masks):
mask = Image.open(mask_path).convert("L")
_require_image_size(mask, image.size)
… follow the assets' orientation_states
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (3)
embodichain/gen_sim/scene_engine/pipeline/generation/scene_understanding.py:71
- The scene-understanding VLM prompt still instructs the model that
namemust not contain spatial words like “left”/“right”, but the parser/tests now accept names containing those words (e.g. "left cup"). This leaves the contract fornameambiguous (prompt says “never”, validation allows it), which can make VLM outputs less consistent and harder to debug.
Consider aligning these by either (a) relaxing the prompt’s name rules/examples to match the accepted schema, or (b) reinstating validation with a narrower rule that only rejects true positional/relational phrasing (while allowing structural words like “top”).
embodichain/gen_sim/scene_engine/pipeline/utils/table_support_surface.py:391
_largest_inscribed_rectangledoes a brute-force search over pairs of unique x/y coordinates withpolygon.covers(rectangle)inside the innermost loop. Even with the 48-point cap, this is up to ~48^4 (~5.3M) rectangle checks, which can be very slow given Shapely predicate costs and will run during table processing.
Consider replacing this with a more efficient heuristic/algorithm (e.g., iterative shrink from bounds, grid search over a much smaller candidate set, or an optimization-based approach), or lowering the search bound further and documenting the accuracy/performance tradeoff.
for x_index, minimum_x in enumerate(x_values[:-1]):
for maximum_x in x_values[x_index + 1 :]:
if maximum_x <= minimum_x:
continue
for y_index, minimum_y in enumerate(y_values[:-1]):
embodichain/gen_sim/scene_engine/pipeline/utils/image_segmentation_utils.py:340
render_asset_mask_id_overlayopens the source image and each mask withImage.open(...).convert(...)but never closes the underlying file handles. In long-running processes or scenes with many assets, this can leak file descriptors.
Prefer with Image.open(...) as img: / as mask: (or explicitly calling .close() after .convert()), especially since this code is used in the main scene-understanding path.
image = Image.open(image_path).convert("RGBA")
overlay = Image.new("RGBA", image.size, (0, 0, 0, 0))
colors = (
(239, 83, 80, 255),
(66, 165, 245, 255),
(102, 187, 106, 255),
(255, 202, 40, 255),
(171, 71, 188, 255),
(38, 198, 218, 255),
)
decoded_masks: list[tuple[str, Image.Image]] = []
for index, (asset_id, mask_path) in enumerate(asset_masks):
mask = Image.open(mask_path).convert("L")
_require_image_size(mask, image.size)
decoded_masks.append((asset_id, mask))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds a text-guided scene editing workflow to Scene Engine.
Main changes
SceneGraphandSceneEditPlancore data structures for tabletop support hierarchies, 9-grid tableregions, planar relations, and edit validation.
--edit_promptsupport:--imageonly: generate a scene.--edit_promptonly: edit an existing valid Scene Engine export.add,move, anddeleteoperations.scene_graph.json,scene.json,category, andname.scene_exportdirectory, copy new assets, and remove deleted assetdirectories.
y, back is smallery.Dependencies: No new Python package dependencies.
Issue reference: N/A
Type of change
Validation
black --check --diff --color .pytest tests/gen_sim/scene_engine -q(73 passed)Screenshots
Not applicable.
Checklist