diff --git a/backend/app/services/plate_thumbnail.py b/backend/app/services/plate_thumbnail.py index a55f2be01..368995a35 100644 --- a/backend/app/services/plate_thumbnail.py +++ b/backend/app/services/plate_thumbnail.py @@ -132,7 +132,17 @@ def _render_model_thumbnails(threemf_bytes: bytes) -> tuple[bytes | None, bytes # Local imports so a `import backend.app.services.plate_thumbnail` from # an environment without matplotlib/trimesh doesn't fail at import time — # the function will simply degrade to no-op via the exception branch. - from backend.app.services.stl_thumbnail import _configure_matplotlib_cache + # + # The light angle is IMPORTED rather than mirrored like the palette above. + # "A plate card and a library thumbnail of the same model look alike" is the + # whole reason these two renderers share a look, and a second copy of the + # angle is exactly how that silently stops being true. A palette can afford a + # copy; a number nobody would notice drifting cannot. + from backend.app.services.stl_thumbnail import ( + _configure_matplotlib_cache, + _repair_winding, + _shade_kwargs, + ) _configure_matplotlib_cache() @@ -141,6 +151,7 @@ def _render_model_thumbnails(threemf_bytes: bytes) -> tuple[bytes | None, bytes matplotlib.use("Agg") import matplotlib.pyplot as plt import trimesh + from matplotlib.colors import LightSource from mpl_toolkits.mplot3d.art3d import Poly3DCollection loaded = trimesh.load(io.BytesIO(threemf_bytes), file_type="3mf", force="mesh") @@ -157,6 +168,15 @@ def _render_model_thumbnails(threemf_bytes: bytes) -> tuple[bytes | None, bytes except Exception as exc: logger.debug("plate_thumbnail: mesh simplification failed, using original: %s", exc) + # Before the vertices are read, not after: ``scaled`` below is indexed by + # ``mesh.faces``, so a repair that ever moves a vertex would leave the two + # out of step. Shared with stl_thumbnail rather than copied — the reason + # these renderers agree is that they run the same code, not similar code. + try: + _repair_winding(mesh, trimesh, "plate_thumbnail") + except Exception as e: # best-effort, as the whole module is + logger.debug("plate_thumbnail: winding repair skipped (%s)", e) + vertices = mesh.vertices bounds_min = vertices.min(axis=0) bounds_max = vertices.max(axis=0) @@ -164,20 +184,36 @@ def _render_model_thumbnails(threemf_bytes: bytes) -> tuple[bytes | None, bytes max_extent = (bounds_max - bounds_min).max() scaled = centered / max_extent if max_extent > 0 else centered + # ndarray, not a list of lists — shading walks this to build normals, and the + # list form is ~30x slower to construct. Paid twice per plate: once per size. faces = mesh.faces - poly3d = [[scaled[v] for v in face] for face in faces] + poly3d = scaled[faces] - large = _render_at_size(poly3d, _PLATE_PNG_SIZE, plt, Poly3DCollection) - small = _render_at_size(poly3d, _PLATE_PNG_SMALL_SIZE, plt, Poly3DCollection) + # Resolved once and shared: both sizes must be lit identically or the 128px + # card and the 512px view disagree. Empty for a mesh matplotlib cannot shade, + # which keeps such a plate rendering flat instead of failing — see + # ``_shade_kwargs``. + shade_kw = _shade_kwargs(poly3d, LightSource) + + large = _render_at_size(poly3d, _PLATE_PNG_SIZE, plt, Poly3DCollection, shade_kw) + small = _render_at_size(poly3d, _PLATE_PNG_SMALL_SIZE, plt, Poly3DCollection, shade_kw) return large, small -def _render_at_size(poly3d, size: int, plt, Poly3DCollection) -> bytes: +def _render_at_size(poly3d, size: int, plt, Poly3DCollection, shade_kw: dict) -> bytes: """Render the prepared poly3d collection to an in-memory PNG.""" + # Local, like every other import in this module, so importing plate_thumbnail + # in an environment without matplotlib still works. stl_thumbnail's own + # module level is import-light, so this costs nothing after the first call. + from backend.app.services.stl_thumbnail import VIEW_AZIM_DEG, VIEW_ELEV_DEG + fig = plt.figure(figsize=(size / 100, size / 100), dpi=100) fig.patch.set_facecolor(_BACKGROUND_COLOR) ax = fig.add_subplot(111, projection="3d") ax.set_facecolor(_BACKGROUND_COLOR) + # ``shade=True`` needs a real ``edgecolors``: matplotlib shades the edge + # colours alongside the face colours, and an empty array (``"none"``) makes + # it raise on the broadcast. Keep the two in step if either moves. ax.add_collection3d( Poly3DCollection( poly3d, @@ -185,12 +221,13 @@ def _render_at_size(poly3d, size: int, plt, Poly3DCollection) -> bytes: edgecolors=_BAMBU_GREEN, linewidths=0.1, alpha=0.9, + **shade_kw, ) ) ax.set_xlim(-0.6, 0.6) ax.set_ylim(-0.6, 0.6) ax.set_zlim(-0.6, 0.6) - ax.view_init(elev=25, azim=45) + ax.view_init(elev=VIEW_ELEV_DEG, azim=VIEW_AZIM_DEG) ax.set_axis_off() ax.grid(False) plt.subplots_adjust(left=0, right=1, top=1, bottom=0) diff --git a/backend/app/services/stl_thumbnail.py b/backend/app/services/stl_thumbnail.py index 846e96512..a883935ee 100644 --- a/backend/app/services/stl_thumbnail.py +++ b/backend/app/services/stl_thumbnail.py @@ -54,6 +54,28 @@ def _configure_matplotlib_cache() -> None: BAMBU_GREEN = "#00AE42" BACKGROUND_COLOR = "#1a1a1a" +# Direction of the synthetic light used to shade the mesh. Without a light +# source ``Poly3DCollection`` fills every triangle with the identical colour +# regardless of its normal, so the render comes out a flat silhouette and one +# model is indistinguishable from another (issue #2816). +# +# The azimuth is NOT free. matplotlib's light direction for (az, alt) is +# ``[cos(90-az)cos(alt), sin(90-az)cos(alt), sin(alt)]``, and the camera set by +# ``view_init(elev, azim)`` sits at ``[cos(elev)cos(azim), cos(elev)sin(azim), +# sin(elev)]``. The dot product of the two must be POSITIVE or the light is +# behind the model: at 225 it is -0.34, which lights the two hidden faces and +# gives both visible ones the identical 0.475 — a cube with no contrast down its +# front edge. At 315 it is +0.30, and the two visible sides come out 0.825 and +# 0.475. ``test_light_is_on_the_camera_side`` holds that invariant so the pair +# cannot drift apart again. +LIGHT_AZIMUTH_DEG = 315 +LIGHT_ALTITUDE_DEG = 45 + +# The camera the light above is chosen against. Named because the two are a PAIR: +# move one without the other and the model goes back to being lit from behind. +VIEW_ELEV_DEG = 25 +VIEW_AZIM_DEG = 45 + # Maximum vertices before simplification MAX_VERTICES = 100000 @@ -66,6 +88,97 @@ MAX_VERTICES = 100000 MIN_USABLE_STL_BYTES = 200 +def _repair_winding(mesh, trimesh, label: str) -> None: + """Make every face wind the same way, and wind it OUTWARD, before shading. + + matplotlib derives its normals from vertex ORDER, so a triangle wound the + wrong way shades as though it faced away and the model comes out patchy — + camouflage rather than a surface. Unshaded this never showed, so lighting the + render is what makes it matter, and the File Manager takes arbitrary user + STLs. ``trimesh.load(force="mesh")`` does not repair winding; this does. + + ``trimesh.repair.fix_winding`` and NOT ``mesh.fix_normals()``: the latter + reaches ``body_count`` -> ``scipy.csgraph``, and scipy is not a dependency of + this project. fix_winding goes through networkx, which requirements.txt + already pins. + + Three steps, because each one leaves something for the next: + + * ``fix_winding`` makes the winding agree but is free to settle on either + orientation, and on a half-inverted sphere it picks INWARD — consistent, + and consistently lit from inside. + * ``fix_inversion`` corrects that off the sign of the volume, but only for a + WATERTIGHT mesh. It returns early otherwise, because a volume measured + across holes says nothing about which way is out. + * Which leaves the common case, since a mesh with broken winding is usually + not watertight either. With no usable volume, decide by whether the faces + point away from the centroid. Measured on a punctured half-inverted + icosphere: the first two steps alone left 0 of 1200 faces oriented like the + correctly wound mesh, a mean render delta of 4.56; with this one it is + 1200 of 1200 and 0.00. + + The centroid test runs only on a mesh whose winding was already broken, and + it leaves correct ones alone: closed and punctured spheres, a flat plate, an + open tube, a non-convex L and two disjoint boxes all sum positive. + + Gated here rather than at the call sites so the two renderers cannot drift. + The check is tens of ms where the repair is seconds on a large mesh, so only + meshes that would otherwise render wrong pay for it. + """ + import numpy as np + + if len(mesh.faces) == 0 or mesh.is_winding_consistent: + return + + logger.debug("Repairing inconsistent winding before render: %s", label) + trimesh.repair.fix_winding(mesh) + trimesh.repair.fix_inversion(mesh) + if mesh.is_watertight: + return + + outward = mesh.triangles.mean(axis=1) - mesh.vertices.mean(axis=0) + if float(np.einsum("ij,ij->i", mesh.face_normals, outward).sum()) < 0: + logger.debug("Winding settled inward on a non-watertight mesh, inverting: %s", label) + mesh.invert() + + +def _shade_kwargs(poly3d, LightSource) -> dict: + """``shade=True`` and its light, or nothing when the mesh cannot be shaded. + + matplotlib's ``_shade_colors`` has a fallback for a mesh whose every face + normal is degenerate, and that fallback returns the colour argument it was + given, unchanged. Passing a colour STRING — which both renderers do — makes + it hand back a 0-d ````. + Worse, ``batch_generate_stl_thumbnails`` walks a whole folder with no + minimum-size pre-skip, so each one would count as a failure in the UI and put + a traceback in the log — the exact noise ``stl_thumbnail``'s demoted logging + exists to keep out. + + Deciding here rather than catching the TypeError keeps the flat render as a + real outcome instead of an error path, and costs ~6 ms on a 227k-face mesh. + Identical to matplotlib's own test: a cross product that is finite and + non-zero for at least one face. + """ + import numpy as np + + if len(poly3d) == 0: + return {} + tri = np.asarray(poly3d, dtype=float) + normals = np.cross(tri[:, 0] - tri[:, 1], tri[:, 1] - tri[:, 2]) + lengths = np.linalg.norm(normals, axis=1) + if not bool(np.any(np.isfinite(lengths) & (lengths > 0))): + return {} + return { + "shade": True, + "lightsource": LightSource(azdeg=LIGHT_AZIMUTH_DEG, altdeg=LIGHT_ALTITUDE_DEG), + } + + def generate_stl_thumbnail( stl_path: Path, thumbnails_dir: Path, @@ -98,6 +211,7 @@ def generate_stl_thumbnail( # Use Agg backend for headless rendering matplotlib.use("Agg") import matplotlib.pyplot as plt + from matplotlib.colors import LightSource from mpl_toolkits.mplot3d import Axes3D # noqa: F401 from mpl_toolkits.mplot3d.art3d import Poly3DCollection @@ -130,6 +244,14 @@ def generate_stl_thumbnail( except Exception as e: logger.warning("Mesh simplification failed, using original: %s", e) + # Wind every face the same way, and outward, or the shading turns the + # model into camouflage. See ``_repair_winding``; it must run before the + # vertices below are read, since a future repair step could move them. + try: + _repair_winding(mesh, trimesh, str(stl_path)) + except Exception as e: # best-effort: a flat render beats no thumbnail + logger.debug("Winding repair skipped (%s): %s", e, stl_path) + # Get mesh bounds and center it vertices = mesh.vertices bounds_min = vertices.min(axis=0) @@ -153,15 +275,24 @@ def generate_stl_thumbnail( ax.set_facecolor(BACKGROUND_COLOR) # Create polygon collection from mesh faces + # Index with the face array rather than building a list of lists. Same + # data, and Poly3DCollection accepts it directly — but shading walks this + # structure to generate normals, and on an 82k-face mesh the list form + # costs ~0.19s against ~0.007s for the ndarray. It speeds up the unshaded + # path too. faces = mesh.faces - poly3d = [[vertices_scaled[vertex] for vertex in face] for face in faces] + poly3d = vertices_scaled[faces] + # ``shade=True`` needs a real ``edgecolors``: matplotlib shades the edge + # colours alongside the face colours, and an empty array (``"none"``) + # makes it raise on the broadcast. Keep the two in step if either moves. collection = Poly3DCollection( poly3d, facecolors=BAMBU_GREEN, edgecolors=BAMBU_GREEN, linewidths=0.1, alpha=0.9, + **_shade_kwargs(poly3d, LightSource), ) ax.add_collection3d(collection) @@ -171,7 +302,7 @@ def generate_stl_thumbnail( ax.set_zlim(-0.6, 0.6) # Set view angle (isometric-ish) - ax.view_init(elev=25, azim=45) + ax.view_init(elev=VIEW_ELEV_DEG, azim=VIEW_AZIM_DEG) # Remove axes and grid ax.set_axis_off() diff --git a/backend/tests/unit/services/conftest.py b/backend/tests/unit/services/conftest.py index 38ed65438..72818833d 100644 --- a/backend/tests/unit/services/conftest.py +++ b/backend/tests/unit/services/conftest.py @@ -1,4 +1,6 @@ -"""Test fixtures for FTP service tests. +"""Shared fixtures for service tests. + +Mostly FTP. Provides a real implicit FTPS server (via mock_ftp_server) and client factory for integration-style testing of BambuFTPClient against a live server. @@ -7,6 +9,7 @@ The server fixture is class-scoped to avoid the overhead of starting a new TLS server for every test (~67 TLS handshakes → ~9 per class). """ +import io import os import shutil import socket @@ -143,3 +146,48 @@ def patch_ftp_port(ftp_server): """ with patch.object(BambuFTPClient, "FTP_PORT", ftp_server.port): yield ftp_server + + +@pytest.fixture() +def distinct_surface_tones(): + """Count the distinct colours covering the model's surface in a render. + + Shared by the STL and plate thumbnail suites, which render the same way + through two different modules and need the same question answered. + + Quantises to 5 bits per channel before counting and keeps only pixels where + green dominates. The spread being quantised away is Agg's antialiasing and + the alpha compositing; PNG itself is lossless and contributes none. + + **This counts large flat tone regions, which is only the same thing as + "is it shaded" for a FLAT-FACED model.** A curved surface produces several + such regions with no light at all — measured unshaded at alpha=0.9: cube 1, + cylinder 1, but sphere 3 and torus 3. So the cube fixture is not incidental; + swap in anything rounder and ``>= 3`` passes on completely unlit output. + A cube is 1 unshaded and 3 lit, and its three margins are comfortable + (0.35 / 0.35 / 0.29, nothing between the noise floor and the threshold). + + Note the green-dominant filter keeps the green-to-background blends along the + silhouette as well as the model — about 1% of the pixels it counts. They sit + far below ``min_share`` individually, so they change no verdict. + """ + + def _count(png: bytes, *, min_share: float = 0.02) -> int: + import numpy as np + from PIL import Image + + # np.asarray, not Image.getdata(): getdata is deprecated for removal in + # Pillow 14 and requirements.txt pins pillow unbounded, while pyproject + # silences DeprecationWarning — so it would surface as an AttributeError + # in CI rather than as a warning anyone saw coming. + rgb = np.asarray(Image.open(io.BytesIO(png)).convert("RGB"), dtype=np.int16) + r, g, b = rgb[..., 0], rgb[..., 1], rgb[..., 2] + surface_mask = (g > r) & (g > b) + if not surface_mask.any(): + return 0 + + keys = ((r >> 3) << 10) | ((g >> 3) << 5) | (b >> 3) + counts = np.bincount(keys[surface_mask].ravel()) + return int((counts / counts.sum() >= min_share).sum()) + + return _count diff --git a/backend/tests/unit/services/test_plate_thumbnail.py b/backend/tests/unit/services/test_plate_thumbnail.py index eeac0443e..5cd4d8b6b 100644 --- a/backend/tests/unit/services/test_plate_thumbnail.py +++ b/backend/tests/unit/services/test_plate_thumbnail.py @@ -135,6 +135,23 @@ class TestInjectPlateThumbnails: assert 480 <= large_w <= 540 and 480 <= large_h <= 540 assert 100 <= small_w <= 140 and 100 <= small_h <= 140 + def test_injected_thumbnail_is_shaded_not_flat(self, distinct_surface_tones): + """Injected plate renders must be lit, same as library thumbnails (#2816). + + The archive card and the File Manager tile show the same model through + two different renderers; if only one of them is lit they disagree. + """ + from backend.app.services.plate_thumbnail import inject_plate_thumbnails_if_missing + + fixture = _build_sliced_3mf(plate_ids=[1], with_thumbnails=set()) + result = inject_plate_thumbnails_if_missing(fixture) + + with zipfile.ZipFile(io.BytesIO(result), "r") as zf: + large = zf.read("Metadata/plate_1.png") + + # _build_sliced_3mf embeds a cube: three faces visible, three tones. + assert distinct_surface_tones(large) >= 3 + def test_injects_for_every_missing_plate_in_multi_plate_3mf(self): """Three plates, plate_2 already has a thumbnail; only plates 1 + 3 get rendered.""" from backend.app.services.plate_thumbnail import inject_plate_thumbnails_if_missing diff --git a/backend/tests/unit/services/test_stl_thumbnail.py b/backend/tests/unit/services/test_stl_thumbnail.py index a2e43934e..3064bfe1d 100644 --- a/backend/tests/unit/services/test_stl_thumbnail.py +++ b/backend/tests/unit/services/test_stl_thumbnail.py @@ -156,6 +156,141 @@ endsolid cube""" # If result is None, dependencies might not be fully functional # which is acceptable + @pytest.mark.skipif( + not _check_trimesh_available(), + reason="trimesh not installed", + ) + def test_generated_thumbnail_is_shaded_not_flat(self, distinct_surface_tones): + """The render must be lit, not a flat silhouette (issue #2816). + + Without ``shade=True`` every triangle is filled with BAMBU_GREEN + regardless of its normal, so any model renders as its own outline and + one file is indistinguishable from another in the File Manager. + """ + import trimesh + + from backend.app.services.stl_thumbnail import generate_stl_thumbnail + + with tempfile.TemporaryDirectory() as tmpdir: + stl_path = Path(tmpdir) / "cube.stl" + trimesh.creation.box(extents=(10.0, 10.0, 10.0)).export(str(stl_path)) + + result = generate_stl_thumbnail(stl_path, Path(tmpdir)) + assert result is not None + + # Three faces of a cube face the camera at the default isometric + # view_init, and with the light on the camera's side each catches it + # differently. This held at 3 before only because ``alpha=0.9`` let a + # back face bleed through two IDENTICALLY lit front faces — re-render + # at alpha=1.0 then and the count fell to 2. It is shading now. + assert distinct_surface_tones(Path(result).read_bytes()) >= 3 + + @pytest.mark.skipif( + not _check_trimesh_available(), + reason="trimesh not installed", + ) + @pytest.mark.parametrize( + ("label", "punch_holes"), + [("watertight", False), ("open", True)], + ) + def test_backwards_wound_triangles_render_the_same(self, label, punch_holes): + """Vertex ORDER must not change the picture. + + matplotlib takes its normals from winding, so an inverted triangle shades + as though it faced away — the model comes out patchy, like camouflage. + Unshaded this was invisible, which makes it a regression the shading + introduced rather than one it revealed, and the File Manager accepts + whatever STL a user uploads. + + Asserted as "same picture as the correctly wound mesh", because the + obvious assertion does not work: broken winding produces MORE distinct + tones, not fewer, so a tone count cannot see it. + + Run BOTH watertight and open, because the two are repaired by different + code. ``fix_inversion`` decides which way is out from the sign of the + volume and gives up when the mesh is not watertight, which is the common + shape of a broken STL — there, ``fix_winding`` settles inward unopposed + and the centroid fallback in ``_repair_winding`` is the only thing + holding this. Without it the open case renders at a mean delta of 4.56. + """ + import numpy as np + import trimesh + from PIL import Image + + from backend.app.services.stl_thumbnail import generate_stl_thumbnail + + sphere = trimesh.creation.icosphere(subdivisions=3, radius=5.0) + keep = sphere.faces.copy()[:-80] if punch_holes else sphere.faces.copy() + good = trimesh.Trimesh(vertices=sphere.vertices.copy(), faces=keep.copy()) + assert good.is_watertight is not punch_holes, "fixture has the wrong topology" + + faces = keep.copy() + faces[::2] = faces[::2][:, ::-1] + bad = trimesh.Trimesh(vertices=sphere.vertices.copy(), faces=faces) + assert not bad.is_winding_consistent, "fixture is supposed to be broken" + + with tempfile.TemporaryDirectory() as tmpdir: + out = Path(tmpdir) + rendered = [] + for name, mesh in (("good", good), ("bad", bad)): + path = out / f"{name}.stl" + mesh.export(str(path)) + result = generate_stl_thumbnail(path, out) + assert result is not None + rendered.append(np.asarray(Image.open(result).convert("RGB"), dtype=float)) + + assert rendered[0].shape == rendered[1].shape + mean_delta = float(np.abs(rendered[0] - rendered[1]).mean()) + + # Repaired they are the same mesh, so this is ~0. Without the repair the + # inverted half renders dark against the lit half and it is an order of + # magnitude higher. + assert mean_delta < 1.0, f"winding changed the {label} render (mean delta {mean_delta:.2f})" + + @pytest.mark.skipif( + not _check_trimesh_available(), + reason="trimesh not installed", + ) + def test_degenerate_mesh_still_renders(self): + """A mesh with no shadeable face must render flat, not fail. + + matplotlib's ``_shade_colors`` falls back to returning the colour it was + given when every normal is degenerate, and for a colour STRING that is a + 0-d array — ``to_rgba_array`` then raises ``TypeError: len() of unsized + object``. So these files rendered fine while the output was flat, and + turning the light on would have broken them. + + They are not hypothetical: stub and truncated STLs reach here, and + ``batch_generate_stl_thumbnails`` walks a whole folder with no + minimum-size pre-skip, so each one would show as a failure in the UI. + """ + import struct + + from backend.app.services.stl_thumbnail import generate_stl_thumbnail + + def write_binary_stl(path, triangles): + # Written by hand rather than via trimesh.export, which drops + # degenerate facets and would quietly defeat the test. + with open(path, "wb") as fh: + fh.write(b"\0" * 80) + fh.write(struct.pack(" 0, "the light is behind the model" + + def shade(normal): + return 0.3 + 0.7 * ((float(np.array(normal) @ light) + 1) / 2) + + # The two faces of an axis-aligned box that face the default camera. + assert abs(shade([1, 0, 0]) - shade([0, 1, 0])) > 0.05, ( + "both visible faces are lit the same — the front edge disappears" + ) + + def test_light_is_above_the_horizon(self): + """Grazing or overhead both collapse the contrast the shading exists for.""" + from backend.app.services.stl_thumbnail import LIGHT_ALTITUDE_DEG + + assert 0 < LIGHT_ALTITUDE_DEG < 90 + def test_background_color(self): """Test that background color is defined.""" from backend.app.services.stl_thumbnail import BACKGROUND_COLOR