Skip to content

feat(rm): allow registering custom radio mat. prior to loading scenes - #71

Open
jeertmans wants to merge 4 commits into
NVlabs:mainfrom
jeertmans:add-register-radio-material
Open

feat(rm): allow registering custom radio mat. prior to loading scenes#71
jeertmans wants to merge 4 commits into
NVlabs:mainfrom
jeertmans:add-register-radio-material

Conversation

@jeertmans

Copy link
Copy Markdown
Contributor

Hi!

While this PR is linked to #70, it does not depend on #70 and should not cause any conflicts.

Currently, loading a scene file containing unknown radio material names or unlisted ITU radio materials raises an error. This presents a challenge in two common scenarios:

  1. Users want to specify new radio material names inside Blender and define their electromagnetic properties in Python code.
  2. The list of ITU materials evolves (e.g., recommendation updates as in feat(radio-materials): update ITU materials to version 4 #70), but Sionna RT's internal list is not yet updated, requiring manual patching of internal dictionaries.

Currently, custom radio materials can only be assigned after loading the scene, which I believe can be error-prone and time-consuming for large scenes.

This PR introduces two helper functions, register_itu_radio_material and register_radio_material, to make registering custom radio materials idiomatic before calling load_scene.

Summary of changes:

  • register_itu_radio_material: Adds register_itu_radio_material(name, parameters, color=None) to allow registering custom ITU materials or updating existing parameters and visual colors in ITU_MATERIALS_PROPERTIES and ITU_MATERIAL_COLORS. If color is omitted or set to None, a random color is generated upon material instantiation, matching RadioMaterial. This changes the previous behavior, which required a color even though it could eventually be overridden.
  • register_radio_material: Adds register_radio_material(rm) and radio_material_registry to register RadioMaterialBase instances prior to scene loading.
  • Scene Preprocessing & Attribute Overrides: Updated process_xml and Scene._load_scene_objects to handle both explicit XML plugin syntax (<bsdf type="itu-radio-material"...> and <bsdf type="radio-material"...>) as well as legacy/Blender syntax (id="itu_..." or id="mat-itu_..."). Any attribute specified in the XML file (such as thickness and color) correctly overrides default or pre-registered material attributes.
  • Multiple Material Customizations: Supports declaring multiple BSDF nodes with distinct IDs (e.g. my_custom_thick_wood and my_custom_thin_wood) referencing the same underlying ITU material type while specifying different property overrides (e.g. thickness).
  • Documentation: Added API documentation for both functions in doc/source/api/radio_materials.rst and comprehensive developer guide examples in doc/source/developer/dev_custom_radio_materials.rst demonstrating explicit XML syntax, material reuse, and attribute overrides.
  • Unit Tests: Added test08_register_itu_radio_material and test09_register_radio_material in test_scene_utils.py covering prefix error validation, explicit vs legacy XML syntax, optional color generation, multiple material IDs referencing the same ITU type, and XML thickness/color overrides.

Let me know if you'd like any adjustments! :-)

@merlinND

merlinND commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Hello @jeertmans,

Thank you for this PR, it looks quite useful! We will review it as well as #70.

Comment thread src/sionna/rt/scene.py Outdated
Comment on lines +950 to +959
registered_rm.color = mi_bsdf.color
registered_rm.thickness = mi_bsdf.thickness
elif hasattr(mi_bsdf, "properties"):
props = mi_bsdf.properties()
for pname in ("color", "reflectance", "base_color"):
if props.has_property(pname):
registered_rm.color = tuple(props[pname])
break
if props.has_property("thickness"):
registered_rm.thickness = float(props["thickness"])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this logic is a bit risky: now any BSDF attached to a shape in the current scene can overwrite properties inside of the radio_material_registry, including for the next shapes in the same scene and future loaded scenes.

It's especially tricky because for ITU materials, "Multiple Material Customizations Referencing the Same ITU Material Type" is explicitly allowed.

IMO, it makes more sense to consider the BSDFs in the registry as immutable and throw an error if an unsupported customization is attempted.

@jeertmans jeertmans Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see! I like having the ability to overwrite some parameters, like colors, directly in the XML file. Otherwise, the parameters could only be set via the registration utilities. However, I agree that this could cause problems if a material is suddenly updated when reading a scene file.

Here are some solutions I have thought of. Let me know which one you prefer, or if you prefer something else.

  • (A) Disallow specifying new RM in the XML file and raise an error if any of the properties differ;
  • (B) Same as (A), but raise a warning instead (and do not overwrite parameters, just let the user know);
  • (C) Allow overwriting radio material properties, but how would you implement this?
  • (D) Same as (C), but do not raise an error if the parameters are set to the same values (just ignore it) and report to the user when the values are different.

It depends on what you mean by

unsupported customization is attempted

:-)

I ended up implementing a version of (C), but scoped by property rather than all-or-nothing: color is purely cosmetic (only read by the renderer/previewer, never touches ray tracing), so it's safe to override per-shape: I do this by returning an independent clone (RadioMaterial.with_color()) rather than mutating the registered instance, so the registry and any other shape using it are untouched. Anything else (thickness, etc.) now raises a clear error telling you to set it on the registered material directly, since silently letting that vary per-shape is exactly the invisible-global-state problem you flagged. Pushed as 0c2d1de, but this required quite some lines of code and possibly breaking changes if you consider process_xml to be public API; let me know what you think.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for giving it a shot!
(A) would be the easiest option but also the most restrictive.
(C) complicates the implementation quite a lot as you saw, but is the most convenient & flexible.

As I think that you noticed, if we go with (C), it is critical for performance that RadioMaterial instances that share identical properties must be referenced in all of the shapes that use it, rather than having one individual copy for each shape. You must also make sure that the merge_shape mechanism still functions as expected when introducing custom radio materials with / without overrides.

In the current implementation, since you introduced a method to clone a radio material, why only allow overriding colors? If the cloning method is correct, then it should be fine to override any property, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point: I actually avoided mutating differentiable attributes, thinking it could cause surprises when trying to optimize a radio material property, and then observe that, because a material a been clone multiple times, the property doesn't get updated on all the clones. However, after checking again, the problem was already present with my solution. So, I went for a more general "clone" solution, but with some documentation about how one just use it when performing differentiation.

I also decided to add a logging message to inform the user about material cloning when loading a scene file, as I believe this could be the main source of "surprises". I set the level to INFO, but let me know if that should be changed. I think Sionna RT is not using a logger anywhere else in its codebase, so I don't know what your position on this.

@jeertmans
jeertmans force-pushed the add-register-radio-material branch from 0c2d1de to 41a20bb Compare August 19, 2026 12:57
@jeertmans
jeertmans force-pushed the add-register-radio-material branch from 41a20bb to 94507b8 Compare August 19, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants