refactor(P1): complete pipeline cleanup — M1 dead code + M3 blender split
M1 dead code removal: - admin.py: remove VALID_STL_QUALITIES + stl_quality (7 locations) - frontend: remove stl_quality from 6 files (api/orders.ts, api/worker.ts, WorkerActivity.tsx, RenderInfoModal.tsx, helpTexts.ts, mocks/handlers.ts) - blender_render.py: delete _mark_sharp_and_seams() — dead, never called (62 lines) - step_processor.py: delete _render_via_service() + 2 elif renderer=="threejs" branches - renderproblems_tmp/: remove 3 orphaned debug images M3 blender_render.py decomposition (858 → 248 lines): - _blender_gpu.py: activate_gpu(), configure_engine() - _blender_import.py: import_glb(), apply_rotation() - _blender_materials.py: FAILED_MATERIAL_NAME, assign_failed_material(), build_mat_map_lower(), apply_material_library() - _blender_camera.py: setup_auto_camera(), setup_auto_lights() - _blender_scene.py: ensure_collection(), apply_smooth_batch(), apply_sharp_edges_from_occ(), setup_shadow_catcher() - Entry-point: sys.path.insert for submodule discovery; arg-parse + Mode A/B orchestration only Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+23
-75
@@ -1,88 +1,36 @@
|
||||
# Review Report: CAD Viewer Material Assignment Fix + Feature Parity
|
||||
Datum: 2026-03-10
|
||||
# Review Report: Pipeline Cleanup (M1 + M3)
|
||||
Date: 2026-03-11
|
||||
|
||||
## Ergebnis: ✅ Freigabe
|
||||
## Result: ✅ Approved (2 low-severity unused imports fixed inline)
|
||||
|
||||
---
|
||||
|
||||
## Gefundene Probleme
|
||||
## Problems Found
|
||||
|
||||
### [InlineCadViewer.tsx + ThreeDViewer.tsx] Misleading comment on isolateMode reset effect
|
||||
**Schwere**: Gering (Kommentar)
|
||||
### render-worker/scripts/blender_render.py:20 — Unused `import math`
|
||||
**Severity**: Low
|
||||
**Description**: `import math` is at the top of the entry-point but `math` is no longer referenced there — all math operations moved to submodules.
|
||||
**Fix**: Remove the import. Applied inline.
|
||||
|
||||
In both files the comment reads:
|
||||
```tsx
|
||||
// Reset isolateMode and hideAssigned when no part is pinned
|
||||
useEffect(() => {
|
||||
if (!pinnedPart) setIsolateMode('none') // ← only resets isolateMode, not hideAssigned!
|
||||
}, [pinnedPart])
|
||||
```
|
||||
The comment says "and hideAssigned" but the effect only calls `setIsolateMode('none')`. The behavior is actually correct — `hideAssigned` should NOT be reset when unpinning (it's a persistent view toggle). Only the comment is wrong.
|
||||
|
||||
**Empfehlung**: Change to `// Reset isolateMode when no part is pinned`.
|
||||
### render-worker/scripts/_blender_import.py:5 — Unused `import re as _re`
|
||||
**Severity**: Low
|
||||
**Description**: `re` module is imported at module level but not used anywhere in `_blender_import.py`. The `_re.sub` calls live in `_blender_materials.py`.
|
||||
**Fix**: Remove the import. Applied inline.
|
||||
|
||||
---
|
||||
|
||||
## Positiv aufgefallen
|
||||
## Positives
|
||||
|
||||
### Bug fix: MaterialPanel invisible in ThreeDViewer — root cause correctly identified
|
||||
The diagnosis was precise: the outer `<div onClick={() => setPinnedPart(null)}>` was receiving the
|
||||
native DOM bubble from every canvas click, calling `setPinnedPart(null)` in the same React batch as
|
||||
`setPinnedPart(name)` from the THREE.js event handler — final state always `null`.
|
||||
|
||||
The two-part fix is clean and idiomatic:
|
||||
- `onClick={(e) => e.stopPropagation()}` on the viewport div absorbs DOM clicks
|
||||
- `onPointerMissed={() => setPinnedPart(null)}` on the R3F Canvas handles the "click empty space"
|
||||
case via the THREE.js raycaster (fires only when no mesh is hit) — this is exactly the right
|
||||
R3F API for this use case
|
||||
|
||||
### cadUtils.ts — normalization regex extension
|
||||
`/_AF\d+(_ASM)?$/i` is minimal and correct. It handles:
|
||||
- `_AF0`, `_AF1` (existing, unchanged)
|
||||
- `_AF0_ASM`, `_AF1_ASM` (new — assembly-node suffix)
|
||||
- Case-insensitive flag is defensive and correct
|
||||
- The loop-until-stable pattern handles nested suffixes as before
|
||||
- `_ASM` alone (without `_AF\d+`) is NOT stripped — correct, it's part of base names like
|
||||
`GE360-HF_000_P_ASM_ASM`
|
||||
|
||||
### Combined visibility useEffect — correct design
|
||||
Merging `hideAssigned` + `isolateMode` into a single traversal effect avoids
|
||||
ordering ambiguity between two independent effects competing on the same `mesh.visible` and
|
||||
`mat.opacity`. The priority order (hideAssigned first, then isolateMode) is explicit and logical.
|
||||
The pinned part (`isSelected`) is always protected from hiding regardless of mode. ✓
|
||||
|
||||
### Effect separation is clean
|
||||
- Color-apply effect: only touches `mat.color` → deps `[modelReady, partMaterials]`
|
||||
- Unassigned glow effect: only touches `mat.emissive` → deps `[modelReady, showUnassigned, partMaterials]`
|
||||
- Combined visibility effect: only touches `mesh.visible` / `mat.opacity` / `mat.transparent` → deps `[modelReady, pinnedPart, isolateMode, hideAssigned, partMaterials]`
|
||||
|
||||
No effect touches another effect's properties — no race conditions.
|
||||
|
||||
### GPU hint and DPR cap
|
||||
`gl={{ powerPreference: 'high-performance' }}` + `dpr={[1, 1.5]}` on both Canvas elements.
|
||||
`preserveDrawingBuffer: true` correctly kept only in ThreeDViewer (required for screenshot capture).
|
||||
|
||||
### "Hide assigned" toolbar button correctly conditional
|
||||
`{assignedCount > 0 && (...)}` in InlineCadViewer and
|
||||
`{modelReady && Object.keys(partMaterials).length > 0 && (...)}` in ThreeDViewer — button only
|
||||
appears when there is something to hide.
|
||||
|
||||
### Debug log is dev-only
|
||||
`if (!import.meta.env.DEV || ...)` guard ensures the console output and traversal overhead
|
||||
never reach production. The output logs both matched and unmatched keys, which is exactly what's
|
||||
needed to diagnose remaining name mismatches after the normalization fix.
|
||||
|
||||
### Feature parity achieved
|
||||
ThreeDViewer and InlineCadViewer now have matching material-assignment features:
|
||||
- ✓ `showUnassigned` highlight toggle with count badge
|
||||
- ✓ `hideAssigned` toggle (new, both viewers)
|
||||
- ✓ `isolateMode` (ghost / hide) via MaterialPanel (both viewers)
|
||||
- ✓ `onPointerMissed` closes panel on empty-space click in ThreeDViewer
|
||||
- **Dead code thoroughly removed**: `VALID_STL_QUALITIES`, `stl_quality` (7 locations in admin.py), 6 frontend files, `_mark_sharp_and_seams()` (62 lines), `_render_via_service()` (33 lines), 2 dead `elif renderer == "threejs"` branches — all gone. All acceptance gates pass.
|
||||
- **Submodule decomposition is clean**: `blender_render.py` went 858 → 249 lines. Each submodule has a clear single responsibility with correct `sys.path.insert(0, ...)` for Blender Python discovery.
|
||||
- **GPU activation order preserved**: `activate_gpu()` still called before `open_mainfile`, and again after engine init — the critical 3-call sequence is intact in `configure_engine()`.
|
||||
- **FailedMaterial sentinel preserved**: `assign_failed_material` in `_blender_materials.py` matches the original logic; unmatched parts in `apply_material_library` are now handled internally.
|
||||
- **`part_names_ordered` global → parameter**: Correctly converted to an explicit parameter in `apply_material_library()`.
|
||||
- **No security issues**: No hardcoded credentials, no SQL injections, no new endpoints, no new models.
|
||||
- **No render pipeline regressions**: No references to removed blender-renderer or threejs-renderer services.
|
||||
- **Frontend**: TypeScript errors in output are pre-existing (Admin.tsx GPUProbeResult, InlineCadViewer.tsx), not introduced by this change.
|
||||
|
||||
---
|
||||
|
||||
## Empfehlung
|
||||
|
||||
**Freigabe.** The one Gering comment issue can be fixed inline.
|
||||
|
||||
Review abgeschlossen. Ergebnis: ✅
|
||||
## Recommendation
|
||||
Approved. Two unused imports fixed inline before commit.
|
||||
|
||||
Reference in New Issue
Block a user