Conversation
fixed quick generation
There was a problem hiding this comment.
Pull request overview
Adds procedural leaf generation and leaf-aware export/distribution features across the C++ core, Python bindings, and Blender add-on.
Changes:
- Introduces C++ leaf generation (shape, venation, presets, LOD) with pybind11 bindings and tests.
- Adds Blender Leaf Shape node + leaf presets + upgraded leaves distribution node group (phyllotaxis, LOD, culling, billboards, normal transfer).
- Extends Pivot Painter (Unreal) export to optionally emit leaf attachment/facing textures when leaf attributes are present.
Reviewed changes
Copilot reviewed 32 out of 35 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_leaf_shape_generator.py | Adds Python integration tests for C++ leaf generator bindings. |
| tests/unit/test_leaf_presets.py | Adds unit tests for Blender-side leaf preset data. |
| python_classes/resources/node_groups.py | Implements v2 leaves distribution node group + procedural default leaf creation. |
| python_classes/presets/leaf_presets.py | Adds Blender UI leaf species preset data structure. |
| python_classes/presets/init.py | Re-exports leaf presets/helpers. |
| python_classes/pivot_painter/formats/unreal.py | Adds optional leaf attachment/facing EXR export path. |
| python_classes/pivot_painter/exporter.py | Detects leaf attributes and enables leaf data export. |
| python_classes/pivot_painter/core.py | Adds computation + pixel packing helpers for leaf attachment/facing textures. |
| python_classes/pivot_painter/init.py | Documents new leaf textures in package docstring. |
| python_classes/operators.py | Adds leaves distribution options + new leaf node operators. |
| python_classes/nodes/tree_function_nodes/tree_mesher_node.py | Adds guard for missing trunk function. |
| python_classes/nodes/tree_function_nodes/leaf_shape_node.py | Adds Leaf Shape node with generation + preset application behavior. |
| python_classes/nodes/tree_function_nodes/init.py | Registers LeafShapeNode. |
| python_classes/nodes/node_categories.py | Adds Leaf category for node editor. |
| python_classes/mesh_utils.py | Adds support for leaf meshes + vein_distance attribute. |
| pyproject.toml | Bumps add-on version to 5.4.0. |
| m_tree/tests/main.cpp | Replaces placeholder with a custom C++ test runner + extensive leaf tests. |
| m_tree/source/tree/Tree.cpp | Throws when executing a tree without a trunk function. |
| m_tree/source/meshers/manifold_mesher/ManifoldMesher.hpp | Adds phyllotaxis_angle attribute name. |
| m_tree/source/meshers/manifold_mesher/ManifoldMesher.cpp | Emits phyllotaxis_angle per cross-section (golden angle progression). |
| m_tree/source/leaf/VenationGenerator.hpp | Adds venation generator API and spatial hash types. |
| m_tree/source/leaf/VenationGenerator.cpp | Implements venation generation + vein_distance attribute computation. |
| m_tree/source/leaf/LeafShapeGenerator.hpp | Adds leaf shape generator API (superformula/margin/venation/deformation). |
| m_tree/source/leaf/LeafShapeGenerator.cpp | Implements contour sampling, triangulation, UVs, venation, deformation. |
| m_tree/source/leaf/LeafPresets.hpp | Adds C++ leaf preset struct and enums. |
| m_tree/source/leaf/LeafPresets.cpp | Adds C++ preset fixtures and lookup helpers. |
| m_tree/source/leaf/LeafLODGenerator.hpp | Adds LOD generator API. |
| m_tree/source/leaf/LeafLODGenerator.cpp | Implements card mesh, billboard cloud, and impostor view directions. |
| m_tree/python_bindings/main.cpp | Exposes leaf APIs + mesh float-attribute helpers to Python. |
| m_tree/pyproject.toml | Bumps core library version to 5.4.0. |
| blender_manifest.toml | Bumps Blender manifest version to 5.4.0. |
| VERSION | Bumps repo VERSION to 5_4_0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Added additional tests, added guards and type checks
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 42 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if billboard_mode != "OFF": | ||
| _billboard_mode_map = {"OFF": 0, "AXIAL": 1, "CAMERA": 2} | ||
| socket_id = _find_socket_identifier(node_group, "Billboard Mode") | ||
| if socket_id is not None: | ||
| modifier[socket_id] = _billboard_mode_map[billboard_mode] |
There was a problem hiding this comment.
The billboard_mode_map dictionary is created inside the conditional block and only used once. This dictionary creation happens every time billboard_mode is not "OFF", which is inefficient. Consider defining this as a module-level constant or class-level constant to avoid repeated dictionary allocation.
| if _pending_timer is not None: | ||
| try: | ||
| bpy.app.timers.unregister(_pending_timer) | ||
| except ValueError: | ||
| msg = f"Failed to unregister pending timer for mesher {mesher.name}" | ||
| print(msg) | ||
| _pending_timer = None |
There was a problem hiding this comment.
The timer unregistration error handling uses a bare print statement instead of proper logging. In a Blender addon, it would be better to use the logging module or at minimum direct the output to sys.stderr to ensure visibility. Additionally, the error is caught but not properly handled - the timer registration continues even if unregistration failed, which could lead to multiple timers running.
| try: | ||
| # Create a simple quad mesh | ||
| mesh = bpy.data.meshes.new(DEFAULT_LEAF_NAME) | ||
|
|
||
| # Define vertices for a simple quad (leaf shape) | ||
| # Oriented so that +Z is "up" from the leaf surface | ||
| verts = [ | ||
| (-0.5, 0.0, 0.0), # bottom left | ||
| (0.5, 0.0, 0.0), # bottom right | ||
| (0.5, 1.0, 0.0), # top right | ||
| (-0.5, 1.0, 0.0), # top left | ||
| ] | ||
| faces = [(0, 1, 2, 3)] | ||
|
|
||
| mesh.from_pydata(verts, [], faces) | ||
|
|
||
| # Add UV coordinates | ||
| uv_layer = mesh.uv_layers.new(name="UVMap") | ||
| uv_data = [(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 1.0)] | ||
| for i, uv in enumerate(uv_data): | ||
| uv_layer.data[i].uv = uv | ||
|
|
||
| mesh.update() | ||
|
|
||
| # Create object | ||
| obj = bpy.data.objects.new(DEFAULT_LEAF_NAME, mesh) | ||
|
|
||
| # Get or create a hidden collection for MTree resources | ||
| collection = bpy.data.collections.get(RESOURCE_COLLECTION_NAME) | ||
| if collection is None: | ||
| collection = bpy.data.collections.new(RESOURCE_COLLECTION_NAME) | ||
|
|
||
| # Context-safe collection linking | ||
| # Check if scene context is available and collection isn't already linked | ||
| scene = ( | ||
| bpy.context.scene | ||
| if bpy.context.scene | ||
| else (bpy.data.scenes[0] if bpy.data.scenes else None) | ||
| ) | ||
| if scene is not None: | ||
| linked_names = [c.name for c in scene.collection.children] | ||
| if RESOURCE_COLLECTION_NAME not in linked_names: | ||
| scene.collection.children.link(collection) | ||
|
|
||
| # Hide the collection in viewport | ||
| collection.hide_viewport = True | ||
| collection.hide_render = True | ||
|
|
||
| collection.objects.link(obj) | ||
|
|
||
| return obj | ||
|
|
||
| return _create_procedural_leaf() | ||
| except Exception: | ||
| # Clean up orphaned resources on failure | ||
| if obj is not None and obj.users == 0: | ||
| bpy.data.objects.remove(obj) | ||
| if mesh is not None and mesh.users == 0: | ||
| bpy.data.meshes.remove(mesh) | ||
| raise | ||
| return _create_quad_leaf() |
There was a problem hiding this comment.
The create_default_leaf_object function silently swallows all exceptions when creating a procedural leaf (line 129), falling back to a quad without logging or notifying the user. This makes debugging difficult if the procedural leaf generation fails. Consider at minimum logging the exception before falling back, so users know why they got a quad instead of a procedural leaf.
| if (nodes[i].parent >= 0) | ||
| { | ||
| nodes[nodes[i].parent].width += nodes[i].width; | ||
| } |
There was a problem hiding this comment.
In compute_pipe_widths, there's a potential out-of-bounds access at line 157. If nodes[i].parent is greater than or equal to nodes.size(), accessing nodes[nodes[i].parent] will be undefined behavior. While parent indices should be valid by design (parent < child), there's no validation to ensure this invariant holds. Consider adding a bounds check: if (nodes[i].parent >= 0 && nodes[i].parent < static_cast<int>(nodes.size())).
WIP - Exploring