Skip to content

Comments

Fixes for geometry parsing, material handling, and centralized asset usage in ISET3D#5

Open
AyushJam wants to merge 19 commits intoISET:masterfrom
AyushJam:fix-geometry
Open

Fixes for geometry parsing, material handling, and centralized asset usage in ISET3D#5
AyushJam wants to merge 19 commits intoISET:masterfrom
AyushJam:fix-geometry

Conversation

@AyushJam
Copy link

@AyushJam AyushJam commented Nov 3, 2025

Hi Brian and ISET Team,

Since the past few months, I've been using ISET3D and ISETCam for a camera simulation project. During this time, with help from you, I identified and fixed some problems in the recipe writing and rendering pipeline in iset3d-tiny. I have described those problems, proposed fixes, and the setup below.

Problems Addressed

PBRT files written out from an ISET recipe, when read back in with piRead create a broken asset tree. I identified that this arises from three main reasons:

  1. Geometric transforms at the highest level in a branch are not saved during recursive parsing in parseGeometryText.m. As a result, when writing out with piWrite, these transforms are missed. This fails rendering.
  2. Mix materials are misread in parseBlockMaterial.m due to double spaces in those strings.
  3. When working with complex scenes with thousands of assets, it is best to place individual assets in a centralized location and refer to it during rendering. The current code has certain checks that throw errors when doing this.

Why

  • My goal was to use the isetauto database and scene recipes from isethdrsensor to develop a simulation framework for camera motion and light control.
  • Doing so required me to fix the above three problems.
  • I have done so and built not just the simulator but also a dataset of renderings.

My Setup

  • Working on Orange directly, I was using a local rendering setup since the assets are on acorn. So I'm not fully familiar with the remote rendering config of a recipe. Wherever my changes could potentially affect it, I've dropped a note below.
  • For a typical example from isetauto, I follow the iset3d convention to save the scene PBRT in iset3d-tiny/data/scenes/. This dir contains the three necessary PBRTs (scene_geometry.pbrt, scene_materials.pbrt, scene.pbrt). Additionally, I save three symlinks (geometry, skymap, textures) pointing to the asset database (in my case, acorn/Resources).
  • piWrite write the recipe out into 3d-tiny/local while copying the symlinks along with modified PBRTs.

Changes

I've grouped the fixes into three, within each group they're in a reducing order of importance.

Geometry

  1. parseGeometryText.m: The geometric transforms at the highest level are missed during line-by-line reading and never make it to the asset tree. I found a convenient solution by saving those transforms in the attributes of the tree root rootAsset. Please note that if you compare the written out PBRT with the one read in, the former will have one extra AttributeBegin/End block for every object (one extra level). This is harmless for rendering, just from recipe handling.

Material Handling

  1. parseBlockMaterial.m: This fixes mix material handling by removing white-space-only tokens created by the strsplit around double spaces. Such double spaces occur in the PBRT line after "string materials". Once parsed right, mix materials are saved and rendered correctly.
  2. piParseObjectName.m: piRead reads each PBRT line one by one. If the line is a PBRT comment like #Meshname:"road_001_Camera_B" #Dimension:[ ] with no dimensions (as is often the case), a parameter in this file res becomes empty and the indexing res(1) throws an error. These comments don't affect rendering but break asset tree building. I've fixed it writing a robust object parser.
  3. piTextureCreate.m: By default it makes texture.invert.value = 'false'. This adds a "bool invert" [false] to every line in the materials pbrt. Rendering works fine even if I set it to empty [].

Centralized Assets

  1. escapeShellArg.m and isSymlink.m: To check if directory is a symbolic link to another location (asset database).
  2. piMaterialText.m and piTextureFileFormat.m : Typically iset3d requires assets to be present in the recipe's output directory. When using symlinks, that is not the case, so I have turned the file existence checks in these modules off, assuming that the asset will certainly exist in the central resource. This is not a clean fix -- ideally, we might have the recipe class have an attribute to store the resource location and use it to check files. I didn't implement it since it changes the core recipe structure.
  3. piWrite.m: As described before, we copy the symlinks from the source to destination (local/). This directory will be used for rendering. I comment out blocks that copy whole assets from source directly.
  4. piRecipeDefault.m: It throws an error if the recipe is pre-downloaded. There is a different case for which this must have been written. Maybe this should be a warning instead.
  5. piTextureText.m: It saves the absolute path of assets in the PBRT written out. This path uses the output directory and conflicts with the actual resources. So I use relative paths compatible with symlinks.

Further Improvements for Robustness

  1. The most important step would be to make the file existence blocks compatible with both symlinks and recipe output dir for complex scenes and backward compatibility. These are [2-5] in the previous section.
  2. While I have used this setup for local rendering, I don't know how the remote renderer is intended to be used for and this affects the same files [2-5].

I hope these fixes improve recipe handling reproducibility.
Happy to discuss further and iterate with maintainers.

Thanks,
Ayush

AyushJam and others added 18 commits August 26, 2025 15:29
@digital-pro
Copy link
Contributor

digital-pro commented Nov 3, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AyushJam @digital-pro