Repository navigation
Conversation
With -DUNION_POLYHEDRON, Union_master draws each non-mask volume as one mcdis_polyhedron instead of priority-clipped lines. Surfaces of box, cylinder, cone, sphere (and Union_mesh in McStas) are triangulated and refined adaptively where r_has_highest_priority disagrees, so parts covered by higher-priority volumes are dropped. Masks stay wireframe. Default (line) display is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…untime Co-Authored-By: Claude Opus 5.5 <[email protected]>
--trace=1 always draws lines. Under --trace=2, display_mode=0 (default) and 2 draw polyhedra, display_mode=1 keeps the line drawing. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ic cost merge_lines_to_draw now appends into a geometrically grown buffer, frees lines_new->lines and leaves lines_new empty. The old version aliased lines_new->lines into an empty master (use after free in draw_circle_with_highest_priority), leaked every merged list and copied the whole master twice per merge, which is quadratic for large meshes. draw_line_with_highest_priority now always frees intersection_list.elements, and frees int_dummy (McStas) / temp_intersection (McXtrace). The McXtrace version also no longer exits when the first merged list of an empty master has no lines. Co-Authored-By: Claude Opus 5.5 <[email protected]>
New parameter bvh_display_layers (default 12). When the mesh BVH has more layers than this, mcdisplay draws the bounding boxes of that BVH layer (and of shallower leaves) instead of every triangle edge; 0 keeps the triangle edges. For the 871k-triangle Stanford dragon (19 layers) this is 24,891 lines instead of 1,307,274, and the mcdisplay-webgl instrument.json shrinks from 276 MB to 5.2 MB. Display only, simulation unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The mesh geometry parameters are now a struct mesh_storage, holding the geometry-mesh-lib gmesh_struct and the BVH display layer, like the box/cylinder/sphere/cone storage structs. geometry_parameters.p_mesh becomes p_mesh_storage, and mesh_display_layer is removed from geometry_struct. No change in behaviour. Co-Authored-By: Claude Opus 5.5 <[email protected]>
- Mesh triangles are now read from p_mesh_storage->mesh (renamed from p_mesh). - union_mcdisplay_polyhedron returns 0 for a mesh with display_layer > 0, and Union_master then draws that volume as lines, so the coarse BVH boxes are kept under --trace=2 instead of triangulating the full mesh. Co-Authored-By: Claude Opus 5.5 <[email protected]>
7 of 8 tasks
Collaborator
|
@willend I have a suspicion that you may be right regarding the polygon emission solving the python tooling bottleneck I was finding, so your polyhedra solution may very well fix the large mesh display issue. I am however wondering if these two solutions should not stay as separate PR's to be merged at each their own pace? They are in my opinion quite different PR's and might benefit from separate review processes. |
Contributor
Author
|
I think they absolutely should stay separate. I only added multiple PR's to double-check for potential technical blockers. :-) |
willend
marked this pull request as draft
October 26, 2026 07:16
Contributor
Author
|
Putting this one in draft |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Combines #2898 (Union volumes as polyhedra in mcdisplay) with #2893 (Union_mesh BVH display for large meshes, by @Lomholy), plus one commit that makes the two work together. If this is merged, #2898 and #2893 can be closed.
Contents
mcdis_polyhedronunder--trace=2.--trace=1always draws lines. The new Union_master parameterdisplay_mode=1keeps lines under--trace=2.2e2dfd3eb,7c4f57357,f444dedc8):merge_lines_to_drawownership/leak/quadratic-cost fix, and the Union_meshbvh_display_layersparameter, which draws BVH boxes instead of triangles for large meshes.654e96c2c, to make them work together:p_mesh_storage->mesh(Union_mesh mcdisplay slow with large meshes - optimize merge_lines_to_draw and add option for drawing coarse meshes #2893 renamesp_mesh; git applies it with no conflicts, but it does not compile without this).display_layer > 0is drawn as the BVH box lines also under--trace=2, instead of triangulating the full mesh.union_mcdisplay_polyhedronreturns 0 and Union_master falls back to lines for that volume.Tested (macOS arm64, clang, conda env mccode-dev)
--trace=2--trace=1bvh_display_layers=3-fsanitize=address,undefinedruns clean under--trace=2.merge_lines_to_drawcaller in McStas and McXtrace is consistent with its new ownership (lines_newis freed and emptied), so nothing is freed twice.Not tested: Linux, Windows/MSVC, a GPU build, the real mcdisplay frontends (webgl, matplotlib, plopp, cad), and the Stanford-dragon-sized meshes that #2893 targets. Union_master_GPU is untouched; it already does not compile against the current union-lib.
🤖 Generated with Claude Code