mitsuba-renderer / mitsuba3

Mitsuba 3: A Retargetable Forward and Inverse Renderer
https://www.mitsuba-renderer.org/
Other
2.07k stars 243 forks source link

[Suggestion] Modify `mi.traverse()` to allow "pinning" scene parameter names to instance ID #1342

Open leroyvn opened 3 weeks ago

leroyvn commented 3 weeks ago

Summary

This is a suggestion to contribute to solving the issue of scene parameters appearing under hard-to-predict names after scene tree traversal (see #508 for context).

The problem

When running this script:

import mitsuba as mi

mi_scene = mi.load_dict(
    {
        "type": "scene",
        "some_bsdf": {"type": "diffuse"},
        "rectangle": {
            "type": "rectangle",
            "bsdf": {
                "type": "ref",
                "id": "some_bsdf",
            },
        },
        "disk": {
            "type": "disk",
            "bsdf": {
                "type": "ref",
                "id": "some_bsdf",
            },
        },
    }
)
print(mi.traverse(mi_scene))

we get the following output:

SceneParameters[
  ----------------------------------------------------------------------------------
  Name                           Flags    Type  Parent
  ----------------------------------------------------------------------------------
  disk.bsdf.reflectance.value    ∂        float UniformSpectrum
  disk.to_world                  ∂, D     Transform4f Disk
  rectangle.to_world             ∂, D     Transform4f Rectangle
]

The problem here is that node names are determined by the scene tree structure, which depends on the order in which objects are processed during scene loading. This, from my understanding, depends on the alphabetical order of the keys in the scene dictionary. In this example, the BSDF appears as a child of "disk", and will appear as a child of "rectangle" if "disk" is renamed "zzz".

This behaviour makes it complicated to infer scene parameter names when assembling scenes from many scene dictionary fragments (typically when building a scene with a generator like we have in Eradiate). I provided a more confusing example in discussion #508.

Proposal

I believe a way to improve the predictibility of node names would be to offer to users the possibility to override node names with the underlying instance's ID. Typically, it seems reasonable in the aforementioned example to expect that the reflectance of some_bsdf can be found as some_bsdf.reflectance.value.

To do so, I suggest two things:

This also results in a more intuitive behaviour when declaring BSDFs, phase functions, etc. as top-level objects in the scene dictionary and referencing them later on.

I experimented with this idea in my project, with the added possibility to restrict node name override using regular expressions passed to name_id_override.

Does such a modification look like a good idea to you?

leroyvn commented 3 days ago

Update: I've been experimenting further and it turns out that this sometimes doesn't work when triggering scene updates. The issue is that upon calling SceneParameters.set_dirty(), reverse scene traversal on an ID-aliased entry will eventually raise because parent node name is inferred from the current node's path. For instance, in the previous example, some_bsdf, which has a depth of 1 (it is a child of one of the shapes) has a "level-0" name. I patched SceneParameters and traverse() to track node name aliases, so that hierarchy climbing can resume from the original path once the top level of the aliased branch has been reached.

This brings up a secondary question: If a node has multiple parents, this is not tracked. This means that all parents except one (that is hard to predict) have to be set as dirty manually. Should this be a source of concern?