Skip to content

Re-enable Python bindings for Properties::Type (fixes the Blender plugin) #1570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2025

Conversation

dvicini
Copy link
Member

@dvicini dvicini commented Apr 16, 2025

This PR re-enables the Python bindings for Properties::Type. The motivation for this is that the Blender importer queries the types of properties (e.g., https://github.com/mitsuba-renderer/mitsuba-blender/blob/5a12adda2d0da3ccf3c125a54e36f7cf57988957/mitsuba-blender/io/importer/emitters.py#L52), which currently is broken.

One minor complication is that the enum is nested in Properties, which itself is variant dependent. However, the enum isn't. I therefore added some logic to detect if the type was already bound, similar to the MI_PY_CHECK_ALIAS macro. I am not sure if this is the best option, but it seems to work.

I also modified the unit tests to exercise the functionality

@dvicini
Copy link
Member Author

dvicini commented Apr 16, 2025

One comment: I am not sure if the binding still leaks as referenced in the original comment. Maybe @njroussel would know how to check this?

@dvicini dvicini force-pushed the properties_bindings_fix branch from f465197 to 793d4af Compare April 16, 2025 14:16
@wjakob
Copy link
Member

wjakob commented Apr 23, 2025

I suspect the leaks are a remnant from an older version of nanobind, where enumerations were done with nb::class_<>, which often caused leaks due to circular references. The change looks good, I will merge it. Unrelated: there is some refactoring to Properties planned (refactor branch) that will likely need some careful attention to the Blender plugin to avoid breaking things.

@wjakob wjakob merged commit a9b4f00 into master Apr 23, 2025
5 checks passed
@njroussel njroussel deleted the properties_bindings_fix branch April 24, 2025 06:01
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