Warnings for wireframe rendering usage, smooth shade meshes by default

yzslab · Jan 5, 2024 · 128d395 · 128d395
1 parent 1317422
commit 128d395
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 25 deletions.
diff --git a/src/viser/_message_api.py b/src/viser/_message_api.py
@@ -16,6 +16,7 @@
 import queue
 import threading
 import time
+import warnings
 from typing import (
     TYPE_CHECKING,
     Callable,
@@ -713,7 +714,7 @@ def add_mesh_simple(
         wireframe: bool = False,
         opacity: Optional[float] = None,
         material: Literal["standard", "toon3", "toon5"] = "standard",
-        flat_shading: bool = True,
+        flat_shading: bool = False,
         side: Literal["front", "back", "double"] = "front",
         wxyz: Tuple[float, float, float, float] | onp.ndarray = (1.0, 0.0, 0.0, 0.0),
         position: Tuple[float, float, float] | onp.ndarray = (0.0, 0.0, 0.0),
@@ -731,8 +732,9 @@ def add_mesh_simple(
             wireframe: Boolean indicating if the mesh should be rendered as a wireframe.
             opacity: Opacity of the mesh. None means opaque.
             material: Material type of the mesh ('standard', 'toon3', 'toon5').
-            flat_shading: Whether to do flat shading. Set to False to apply smooth
-                shading.
+                This argument is ignored when wireframe=True.
+            flat_shading: Whether to do flat shading. This argument is ignored
+                when wireframe=True.
             side: Side of the surface to render ('front', 'back', 'double').
             wxyz: Quaternion rotation to parent frame from local frame (R_pl).
             position: Translation from parent frame to local frame (t_pl).
@@ -741,6 +743,16 @@ def add_mesh_simple(
         Returns:
             Handle for manipulating scene node.
         """
+        if wireframe and material != "standard":
+            warnings.warn(
+                f"Invalid combination of {wireframe=} and {material=}. Material argument will be ignored.",
+                stacklevel=2,
+            )
+        if wireframe and flat_shading:
+            warnings.warn(
+                f"Invalid combination of {wireframe=} and {flat_shading=}. Flat shading argument will be ignored.",
+                stacklevel=2,
+            )
 
         self._queue(
             _messages.MeshMessage(

diff --git a/src/viser/client/src/WebsocketInterface.tsx b/src/viser/client/src/WebsocketInterface.tsx
@@ -171,16 +171,20 @@ function useMessageHandler() {
                   message.plane == "xz"
                     ? new THREE.Euler(0.0, 0.0, 0.0)
                     : message.plane == "xy"
-                    ? new THREE.Euler(Math.PI / 2.0, 0.0, 0.0)
-                    : message.plane == "yx"
-                    ? new THREE.Euler(0.0, Math.PI / 2.0, Math.PI / 2.0)
-                    : message.plane == "yz"
-                    ? new THREE.Euler(0.0, 0.0, Math.PI / 2.0)
-                    : message.plane == "zx"
-                    ? new THREE.Euler(0.0, Math.PI / 2.0, 0.0)
-                    : message.plane == "zy"
-                    ? new THREE.Euler(-Math.PI / 2.0, 0.0, -Math.PI / 2.0)
-                    : undefined
+                      ? new THREE.Euler(Math.PI / 2.0, 0.0, 0.0)
+                      : message.plane == "yx"
+                        ? new THREE.Euler(0.0, Math.PI / 2.0, Math.PI / 2.0)
+                        : message.plane == "yz"
+                          ? new THREE.Euler(0.0, 0.0, Math.PI / 2.0)
+                          : message.plane == "zx"
+                            ? new THREE.Euler(0.0, Math.PI / 2.0, 0.0)
+                            : message.plane == "zy"
+                              ? new THREE.Euler(
+                                  -Math.PI / 2.0,
+                                  0.0,
+                                  -Math.PI / 2.0,
+                                )
+                              : undefined
                 }
               />
             </group>
@@ -279,7 +283,8 @@ function useMessageHandler() {
           wireframe: message.wireframe,
           transparent: message.opacity !== null,
           opacity: message.opacity ?? 1.0,
-          flatShading: message.flat_shading,
+          // Flat shading only makes sense for non-wireframe materials.
+          flatShading: message.flat_shading && !message.wireframe,
           side: {
             front: THREE.FrontSide,
             back: THREE.BackSide,
@@ -290,19 +295,19 @@ function useMessageHandler() {
           throw new Error(`Should never get here! ${x}`);
         };
         const material =
-          message.material == "standard"
+          message.material == "standard" || message.wireframe
             ? new THREE.MeshStandardMaterial(standardArgs)
             : message.material == "toon3"
-            ? new THREE.MeshToonMaterial({
-                gradientMap: generateGradientMap(3),
-                ...standardArgs,
-              })
-            : message.material == "toon5"
-            ? new THREE.MeshToonMaterial({
-                gradientMap: generateGradientMap(5),
-                ...standardArgs,
-              })
-            : assertUnreachable(message.material);
+              ? new THREE.MeshToonMaterial({
+                  gradientMap: generateGradientMap(3),
+                  ...standardArgs,
+                })
+              : message.material == "toon5"
+                ? new THREE.MeshToonMaterial({
+                    gradientMap: generateGradientMap(5),
+                    ...standardArgs,
+                  })
+                : assertUnreachable(message.material);
         geometry.setAttribute(
           "position",
           new THREE.Float32BufferAttribute(