Skip to content

Conversation

as-bw
Copy link

@as-bw as-bw commented May 29, 2025

Motivation

This pull request introduces significant enhancements to the nicegui library's 3D scene functionality, including new object types, hover event handling, and expanded customization options for meshes. The changes improve interactivity, object rendering, and integration with CadQuery to render 3D CAD geometry. Below is a summary of the most important changes grouped by theme.

Implementation

The following key changes have been implemented:

  • nicegui/elements/scene_objects.py:

    • New Mesh Object:
      Allows rendering of user defined meshs given by vertices and triangles matrices. These mesh data can for example come from a mesh library like e.g. https://trimesh.org/.

    • New FacetMesh Object:
      This is an extension of the Mesh object where you can also define multiple triangles belonging to one facet. A facet is a continuous surface that is to be displayed without edges. This context also exists for example in trimesh.

    • New CQShape Object:
      Allows rendering of individual CadQuery Shape (https://cadquery.readthedocs.io/en/latest/) objects.
      Converts CQ shapes to FacetMesh for display.
      Supports wireframe (renders only edges) and show_edges (renders solid with edges) parameters.

    • New CQAssembly Object:
      Enables rendering of CadQuery Assembly objects.
      Recursively processes and renders child shapes and nested assemblies, applying their respective locations and colors.
      Propagates wireframe and show_edges settings to its constituent parts.

  • nicegui/examples/3d_scene/main.py:
    Updated the example to show the new types of geometries. Also included a hover handler to color facets that are hovered. Maybe from that more functionality can be derived in future.

  • For the mesh rendering I added a 'mesh' type to the scene.js

  • For the hover handling of the faces/facets i derived functionality from the click handler in the following files:

    • events.py
    • scene.py
    • scene.js (mouseMove)
    • scene_view.js (mouseMove)

Progress

  • The implementation is completed and working as shown in examples/3d_scene/main.py
  • Unfortunatly I have no time/experience to write pytests for that. I tested directly with the example.

Documentation Updates

  • Updated the Scene class docstring to reflect the expanded support for 3D objects, including CadQuery integration and new mesh types. ([[1]](https://github.com/zauberzeug/nicegui/pull/4810/files#diff-a345688ca0d5cde0c1eac68459a1b81fdb898c9e61237f389c6dd89030d9cff1L89-R99), [[2]](https://github.com/zauberzeug/nicegui/pull/4810/files#diff-a345688ca0d5cde0c1eac68459a1b81fdb898c9e61237f389c6dd89030d9cff1R108-R110))

These changes collectively improve the library's ability to create interactive and visually rich 3D scenes, making it more versatile for developers working with complex geometries and user interactions.

as-bw added 3 commits May 29, 2025 12:05
- Reformatted the SceneView class for better code style consistency, including spacing and alignment.
- Updated property access in SceneClickEventArguments and related classes to use double quotes for string literals.
- Enhanced the handle_event function to improve readability by restructuring conditional checks.
- Removed commented-out code and unnecessary comments for cleaner code.
@evnchn evnchn requested a review from Copilot May 29, 2025 22:07
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enriches the 3D scene API in NiceGUI by introducing custom mesh types and CadQuery integration, plus hover event support.

  • Added Mesh, FacetMesh, CQShape, and CQAssembly for code-defined and CAD meshes
  • Implemented hover event handling across Python and JS layers
  • Updated examples to showcase new mesh types and hover behavior

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
pyproject.toml Added quote-style = "single" for Ruff formatting
nicegui/events.py Defined SceneHoverEventArguments
nicegui/elements/scene_view.py Reformatted string quotes, bound hover handler
nicegui/elements/scene.js Registered hover3d events and props
nicegui/elements/scene_objects.py Introduced Mesh, FacetMesh, CQShape, CQAssembly
nicegui/elements/scene.py Added on_hover API and hover event plumbing
examples/3d_scene/main.py Demonstrated hover highlighting in example

self.wireframe = wireframe


class FacetMesh(Object3D):
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

The type hint for the facets parameter is Optional[int], but it should be something like Optional[List[List[int]]] to match the docstring and expected data structure.

Copilot uses AI. Check for mistakes.

@@ -95,7 +105,9 @@ def __init__(self,
:param grid: whether to display a grid (boolean or tuple of ``size`` and ``divisions`` for `Three.js' GridHelper <https://threejs.org/docs/#api/en/helpers/GridHelper>`_, default: 100x100)
:param camera: camera definition, either instance of ``ui.scene.perspective_camera`` (default) or ``ui.scene.orthographic_camera``
:param on_click: callback to execute when a 3D object is clicked (use ``click_events`` to specify which events to subscribe to)
:param on_hover: callback to execute when a 3D object is clicked (use ``click_events`` to specify which events to subscribe to)
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

The docstring for on_hover still refers to clicking and click_events; update to describe hover behavior and use of hover_events.

Copilot uses AI. Check for mistakes.

pyproject.toml Outdated
@@ -167,6 +167,9 @@ exclude = [
"website/documentation/content/*",
]

[tool.ruff.format]
quote-style = "single"
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

[nitpick] Code changes introduce many double-quoted strings, but Ruff is configured for single quotes. Align the code or the config to keep quotes consistent.

Suggested change
quote-style = "single"
quote-style = "double"

Copilot uses AI. Check for mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

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

On the current main branch there is already a Ruff setting for single quotes:

[tool.ruff.lint.flake8-quotes]
inline-quotes = "single"
multiline-quotes = "single"
docstring-quotes = "double"
avoid-escape = true

The PR should follow the convention of single quotes for strings.

shift_key: mouseMove.shiftKey,
});
};
this.$el.onmousemove = hover_handler;
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

Rather than hard-coding onmousemove and onmouseenter, reuse the hover_events prop (like this.hover_events.forEach(...)) to allow custom event configuration and ensure onmouseleave is also handled.

Copilot uses AI. Check for mistakes.

if (
child_obj is None
): # Indicates a nested assembly (assembly_item.obj is None, assembly_item itself is the cq.Assembly)
nested_assembly_obj = self.scene.cq_assembly(
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

The nested assembly object is created but never added to the parent group or scene hierarchy—consider calling an add or using context to attach it so it actually renders.

Copilot uses AI. Check for mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like Copilot doesn't understand that self.scene.cq_assembly already adds it to the scene.

@@ -87,6 +87,29 @@ export default {
this.$el.onclick = click_handler;
this.$el.ondblclick = click_handler;

const hover_handler = (mouseMove) => {
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

[nitpick] Running a full raycast on every mouse move can impact performance in complex scenes—consider throttling the handler or debouncing the intersection checks.

Copilot uses AI. Check for mistakes.

Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Thanks for this pull request, @as-bw!

It will take some time for us to review it in detail. According to its description, it seems to be a rather big one with many new features (line count: +894 −225). Often it's better to break it into smaller, independent PRs so that one controversial feature doesn't block another more straightforward one. But we'll see if this is the case for your PR.

Just a quick feedback after skimming over the code and Copilot's remarks:
You seem to be using a different formatter (black?) which changes indentation and quotations. Please have a look into the formatting section of our contribution guidelines and adjust the code accordingly. A smaller diff will better preserve the Git history and allows for a quicker, more efficient review. (As a rule of thumb: I'm getting suspicious if there are a lot of red lines in the diff, meaning you either removed or changed large parts of existing code.)

if (
child_obj is None
): # Indicates a nested assembly (assembly_item.obj is None, assembly_item itself is the cq.Assembly)
nested_assembly_obj = self.scene.cq_assembly(
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like Copilot doesn't understand that self.scene.cq_assembly already adds it to the scene.

pyproject.toml Outdated
@@ -167,6 +167,9 @@ exclude = [
"website/documentation/content/*",
]

[tool.ruff.format]
quote-style = "single"
Copy link
Contributor

Choose a reason for hiding this comment

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

On the current main branch there is already a Ruff setting for single quotes:

[tool.ruff.lint.flake8-quotes]
inline-quotes = "single"
multiline-quotes = "single"
docstring-quotes = "double"
avoid-escape = true

The PR should follow the convention of single quotes for strings.

@falkoschindler falkoschindler changed the title Feat: Enhanced 3D Mesh Rendering by allowing for code defined meshes from trimesh & Integrate CadQuery 3D Geometry Let ui.scene support trimeshes and CadQuery 3D geometries May 30, 2025
@falkoschindler falkoschindler added feature Type/scope: New feature or enhancement in progress Status: Someone is working on it labels May 30, 2025
@as-bw as-bw requested a review from falkoschindler June 4, 2025 22:10
@as-bw
Copy link
Author

as-bw commented Jun 4, 2025

I tried to adress all important reviews.

@as-bw
Copy link
Author

as-bw commented Jun 4, 2025

Thanks for this pull request, @as-bw!

It will take some time for us to review it in detail. According to its description, it seems to be a rather big one with many new features (line count: +894 −225). Often it's better to break it into smaller, independent PRs so that one controversial feature doesn't block another more straightforward one. But we'll see if this is the case for your PR.

Just a quick feedback after skimming over the code and Copilot's remarks: You seem to be using a different formatter (black?) which changes indentation and quotations. Please have a look into the formatting section of our contribution guidelines and adjust the code accordingly. A smaller diff will better preserve the Git history and allows for a quicker, more efficient review. (As a rule of thumb: I'm getting suspicious if there are a lot of red lines in the diff, meaning you either removed or changed large parts of existing code.)

I tried to follow the instructions in formatting section this led my to the wrong formatting. If I had not followed this section, there were way less problems. Unfortunatly, I am not experienced with these formatting tools. In the end it took a lot of time to get these tools running and then it led to wrong results :/
I definetly did something wrong there. Nevertheless, I hope that the end result is fine.

@evnchn
Copy link
Collaborator

evnchn commented Jun 5, 2025

Hello @as-bw

Do you mind not modifying the existing examples/3d_scene/main.py, but put your new example in another folder?

That would make things more clear and provide more options, as people can refer to the old example if they just want a very simple 3D scene, or use the more complex and feature-rich one we’re adding in this PR.

Also, generally changing the existing example to something else is not very good practice, as it can lead to confusion.

@as-bw
Copy link
Author

as-bw commented Jun 5, 2025

Hello @as-bw

Do you mind not modifying the existing examples/3d_scene/main.py, but put your new example in another folder?

That would make things more clear and provide more options, as people can refer to the old example if they just want a very simple 3D scene, or use the more complex and feature-rich one we’re adding in this PR.

Also, generally changing the existing example to something else is not very good practice, as it can lead to confusion.

I understand the esence of your point, but I thought there are enough simple examples about the 3d scene in the documentation: https://nicegui.io/documentation/scene#scene_view. I didn't really get the point of having examples in the documentation and in the exmaple folder. So I didn't get the importance of the examples of the examples folder, I tought these are some old artefacts.

Any preferences on the way this new example for the more complex 3d scene is structured, integrated and named?
Because otherwise if I decide on these points it will probably not meet the requirements again and I have to change it again.

@falkoschindler
Copy link
Contributor

@as-bw I'm sorry for the trouble you had with the code formatting. Please let us know if there are specific instructions that need correction or improvement.

Regarding the examples folder: It contains example apps that are hard to show in an interactive demo. The 3D scene example is an edge case, because we could have shown it as a demo as well. I think we hesitated because you need to serve an extra STL file which we didn't want to do for the NiceGUI website at that time.

Anyway, I'd suggest I'll do a quick review before you continue changing the implementation (or example). With a better understanding of the new features and showcases we might find a way to break it down or even move it into small independent demos.

@falkoschindler falkoschindler added review Status: PR is open and needs review and removed in progress Status: Someone is working on it labels Jun 6, 2025
@falkoschindler falkoschindler self-assigned this Jun 6, 2025
Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Over the weekend I spent some time with this PR, especially with the new example, and I came to the following conclusion.

This PR contains three features:

  1. scene.mesh and scene.facetmesh,
  2. scene.cq_shape and scene.cq_assembly (based on scene.facetmesh), and
  3. scene.on_hover (independent on the other two features).
    Because 1. and 2. are very close, I agree that they can remain in a single PR. But hovering is a separate topic which would deserve its own PR or feature request.

At the moment my main concern about on_hover is - as noticed by GitHub Copilot - that every mouse move will cause a raycast which can get pretty costly for complex scenes. This topic needs some discussion and possibly a more complex solution, which is why I would like to separate it from this PR.

Likewise, the example basically splits into three parts: hovering, meshes and CAD Query objects. The latter boil down to something like

vertices = [[0, 0, 0], [1, 0, 0], [1, 1, 0], [0, 1, 0],
            [0, 0, 1], [1, 0, 1], [1, 1, 1], [0, 1, 1]]
triangles = [[1, 0, 2], [2, 0, 3],
             [4, 5, 6], [4, 6, 7],
             [0, 1, 5], [0, 5, 4],
             [2, 3, 7], [2, 7, 6],
             [1, 2, 6], [1, 6, 5],
             [3, 0, 7], [7, 0, 4]]
facets = [[0, 1], [2, 3], [4, 5], [6, 7], [8, 9], [10, 11]]

with ui.scene(width=1024, height=800) as scene:
    scene.mesh(vertices, triangles) \
        .move(x=-3.5, y=-1, z=1).material('red')
    scene.mesh(vertices, triangles, wireframe=True, threshold_angle=45) \
        .move(x=-1.5, y=-1, z=1).material('green')
    scene.mesh(vertices, triangles[:6], wireframe=True, only_show_open_edges=True, threshold_angle=0) \
        .move(x=0.5, y=-1, z=1).material('blue')
    scene.mesh(vertices, triangles[:6], wireframe=True, only_show_open_edges=False, threshold_angle=0) \
        .move(x=2.5, y=-1, z=1).material('cyan')

    scene.facetmesh(vertices, triangles, facets, show_edges=True) \
        .move(x=-1.5, y=1, z=1).material('orange').edge_material('black')
    scene.facetmesh(vertices, triangles, facets, show_edges=False) \
        .move(x=0.5, y=1, z=1).material('pink')

and

box_with_hole = cq.Workplane('XY').box(1, 1, 1).faces('>Z').workplane().hole(0.5).val()

assembly = cq.Assembly()
assembly.add(cq.Workplane('XY').sphere(radius=0.5).val(), color=cq.Color('cyan'))
assembly.add(cq.Workplane('XY').cylinder(height=2, radius=0.25).val(), color=cq.Color('yellow'))

with ui.scene(width=1024, height=800) as scene:
    scene.cq_shape(box_with_hole) \
        .move(x=-2, y=-1, z=1).material('red')
    scene.cq_shape(box_with_hole, wireframe=True) \
        .move(x=0, y=-1, z=1).edge_material('green')
    scene.cq_shape(box_with_hole, wireframe=False, show_edges=True) \
        .move(x=2, y=-1, z=1).material('blue').edge_material('white')

    scene.cq_assembly(assembly).move(x=1, y=1, z=1)
    scene.cq_assembly(assembly, wireframe=False, show_edges=True).move(x=1, y=1, z=1)

and should be converted into two demos on the ui.scene documentation page.

Note that .move() doesn't seem to work for assemblies, which is unexpected and probably needs to be fixed.

To move forward, I propose to

  • remove on_hover from this PR,
  • revert the example and create two demos instead,
  • check if .move() can be fixed for assemblies.

@falkoschindler falkoschindler added in progress Status: Someone is working on it and removed review Status: PR is open and needs review labels Jun 10, 2025
@falkoschindler falkoschindler removed their assignment Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Type/scope: New feature or enhancement in progress Status: Someone is working on it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants