Conversation
|
Hi @Speierers, absolutely spectacular! I'm not sure to what extent we can really review this as a normal PR. Still, it would be great to have @bathal1's thoughts. One thing that concerns me is the full loss of the testing harness. Experience has shown that projects without one end up unhealthy. Ideally we would have at least basic coverage to ensure that nothing breaks catastrophically. Is that something you could still add? PS: I am very curious how UDIMs in Mitsuba can work 😆 |
Wouldn't it be enough to revive the previous test suite for this refactored codebase? The testing infrastructure is still here, it is just a matter to getting it to work again. |
|
I would like to at least test rendering of some basic scenes to avoid catastrophic breakage. I am actually not sure what the previous test suite covered (@bathal1 , do you know?) |
|
What's the status on this? I am increasingly interested in improvements to the blender exporter and was wondering if we are expecting to merge a version of this? What is missing? |
|
Hi @dvicini , I've been looking into this and will be close to updating the PR with some more (minor) changes/additions soon. |
bathal1
left a comment
There was a problem hiding this comment.
Thank you very much for the PR @Speierers, this is very cool work !
Here is my review, you will find several comments/questions, it would be great if you could clarify them.
My main concern with this PR is that it seems to heavily focus on rendering inside blender, at the expense of exporting. In particular, having python implementation of various Cycles nodes as Mitsuba plugins is great to render blender native scenes, but I am concerned that this will be misleading for users who want to export a scene to XML, where some materials may be exported without raising errors, and later fail when loading the scene in Mitsuba as some plugins aren't implemented. Some functionality should be added to make sure this doesn't happen, as exporting to XML for later use is still AFAIK the main use case for this addon.
A more minor comment: I did not see any remapping of roughness in the new material export code, was that somehow fixed? Blender usually squares the roughness value that the user inputs so some remapping is necessary to make renderings with Mitsuba match Blender.
To merge this PR, we would also need to:
- Add the test suite back, which includes round-trip tests to make sure things are consistent
- Add support for installing mitsuba with pip, as this is the preferred way
- Merge the correesponding PR on the Mitsuba 3 repo.
| if radius > 1e-3: | ||
| return { | ||
| 'type': 'spherelight', | ||
| 'position': position, | ||
| 'radius': radius, | ||
| 'intensity': intensity, | ||
| 'soft_falloff': b_light.data.use_soft_falloff | ||
| } |
There was a problem hiding this comment.
This seems dangerous. IIUC it will only work when rendering inside blender, as the corresponding mitsuba plugin will be registered. However, once the scene is exported to XML for downstream applications, this will result in an error as the spherelight plugin does not exist in Mitsuba.
| # for color_layer in b_mesh.vertex_colors: | ||
| # if color_layer.name in b_mesh.attributes: | ||
| # props[f'vertex_{color_layer.name}'] = b_mesh.attributes[color_layer.name].data[0].as_pointer() | ||
| # else: | ||
| # props[f'vertex_{color_layer.name}'] = color_layer.data[0].as_pointer() |
| mesh_dict = convert_mesh(ctx, b_mesh, transform, name_clean, 0) | ||
| converted_parts.append((-1, mesh_dict)) |
There was a problem hiding this comment.
There used to be a sanity check that the mesh was converted correctly and non-empty, why remove it ?
| if displacement: | ||
| mesh_dict['displacement'] = displacement | ||
|
|
||
| if mesh_dict is not None: |
There was a problem hiding this comment.
The mesh dict could also contain an empty mesh, which we shouldn't export
| entry = mi.Bitmap(image.filepath_raw) | ||
| else: | ||
| key = 'filename' | ||
| # TODO: don't save packed images but convert them to a mitsuba texture, and let the XML writer save |
There was a problem hiding this comment.
This TODO is tken care of by the code above, right ?
| def find_mitsuba(): | ||
| ''' | ||
| Find Mitsuba path by importing Mitsuba on the system Python (python or python3) | ||
| ''' | ||
| def f(executable): | ||
| import subprocess | ||
| cmd = [executable, '-c', 'import mitsuba; print(mitsuba.__path__[0])'] | ||
| return subprocess.run(cmd, capture_output=True, text=True).stdout[:-5] | ||
| try: | ||
| return f('python') | ||
| except: | ||
| try: | ||
| return f('python3') | ||
| except: | ||
| return '' |
| uvindices = (el.index(b's'), el.index(b't')) | ||
| if -1 in uvindices: | ||
| uvindices = (el.index(b'u'), el.index(b'v')) | ||
| if True: |
There was a problem hiding this comment.
This should probably be removed
| if mi_texture.has_property('gain'): | ||
| bl_mixnode = bl_mat_wrap.ensure_node_type([out_socket_id], 'ShaderNodeMixRGB', 'Color') | ||
| bl_mixnode.blend_type = 'MULTIPLY' | ||
| bl_mixnode.inputs[0].default_value = 1.0 # Factor | ||
| bl_mixnode.inputs[2].default_value = bl_shader_utils.rgb_to_rgba([mi_texture.get('gain')] * 3) | ||
| bl_teximage = bl_mat_wrap.tree.nodes.new(type='ShaderNodeTexImage') | ||
| bl_mat_wrap.tree.links.new(bl_teximage.outputs[0], bl_mixnode.inputs[1]) | ||
| else: | ||
| bl_teximage = bl_mat_wrap.ensure_node_type([out_socket_id], 'ShaderNodeTexImage', 'Color') | ||
|
|
There was a problem hiding this comment.
Mitsuba textures don't have a gain property. Maybe this was meant to be deleted ?
| def mi_area_to_bl_light(mi_context, mi_emitter, mi_props_id): | ||
| bl_light = bpy.data.lights.new(name=mi_props_id, type='AREA') | ||
| color, strength = _get_radiance_value(mi_context, mi_emitter, 'radiance', [1.0, 1.0, 1.0]) | ||
| bl_light.color = color | ||
| bl_light.energy = strength | ||
| world_matrix = Matrix() | ||
| return bl_light, mi_context.mi_space_to_bl_space(world_matrix) |
There was a problem hiding this comment.
This should probably be merged with the function below, it doesn't seem to be used anywhere else
There was a problem hiding this comment.
Why is this not directly defined in the MitsubaRenderEngineclass ?
|
I added the default pip install option back. @Speierers I'm interested to hear if you tested the reload of mitsuba when changing the custom path. In my experience it is a huge pain to point to a new build folder, and I couldn't get it to work today. I reverted to requiring the user to restart blender whenever such a change is made. |
In general, Blender 3.6 LTS will stop being supported June 2025 so clean-up export/import to focus on v4.0+ * Remove curves export as uses currently uses unsupported shape plugin * Add translucent BSDF export support * Clean-up export principled BSDF * Clean-up export of normal/bump BSDF * Added brightness/contrast texture plugin * Added invert_rgb texture plugin * Added mix_rgb texture plugin * Added color_ramp texture plugin
- Added a checkbox to toggle between pip version and custom path
- Require a restart whenever the mitsuba path is changed
- Added messages for common errors
6c53584 to
ce07ab0
Compare
e8dff3e to
7d0451a
Compare
80748f6 to
a5e7a60
Compare
8a61844 to
a180421
Compare
|
Hi @bathal1 and @Speierers, Leander Cretigny will work on the blender plugin this semester. To me, the most important thing seems to be to merge this PR and add proper support for testing. Do you agree with this assessment? Is there any feedback or suggestions you would have for him? |
|
Baptiste and I had this WIP PR that updates the plugin to the latest Mitsuba version and were blocked due to some Windows crashes that we didn't have time to investigate: #137 The PR also improves the testing setup and instructions a bit. Maybe it would be nice to first do an incremental update (as done in PR #137), and then adapt all these larger changes? |
Major refactoring
This pull request introduces a significant refactoring of the Blender add-on, bringing substantial improvements in performance, usability, and features. The changes are designed to enhance the overall user experience, making it more efficient and enjoyable to work with Mitsuba in Blender.
This new version of the add-on relies on some changes to the Mitsuba changes, introduced in this PR
Overview video
mitsuba_blender_example.mov
Key Enhancements
Mix,math,separate_rgb,hue_saturation,map_range,clamp,rgb_to_bw,combine_color, andnoisetexture shader nodes💥🔪 The sharp bits 🔪💥