Union_mesh mcdisplay slow with large meshes - optimize merge_lines_to_draw and add option for drawing coarse meshes by Lomholy · Pull Request #2893 · mccode-dev/McCode

Lomholy · 2026-10-23T19:27:28Z

Free-form text area

Please describe what your PR is adding in terms of features or bugfixes:

Following the merge of #2866 I wanted to do a quick simulation with a big mesh to showcase how cool this new addition is. I ran into the issue that I could not use mcdisplay to visualize my geometry.
This PR fixes that by doing two things for the Union_mesh visualization. First it optimizes the allocation procedure in the merge_lines_to_draw and an int array leak in r_has_highest_priority functions, because those were causing issues with memory.
This fix however is not enough to draw a Stanford Dragon (871414 triangles) on my M4 max laptop.

So the second fix is to use the Bounding volume hierarchy to allow the user to see a coarser approximation of their shape. The parameter bvh_display_layers has been added to the Union_mesh component for this reason. When set to an integer, the integer decides what level of the Bounding volume hierarchy will be displayed. Higher bvh_display_layers, more detail.

A screenshot from the mcdisplay with the stanford dragon and a bvh_display_layers = 14:

Declaration of use of AI-tools

Please add a checkmark here if you used AI-tools during the work for this contribution
Further, please describe how / where and for what the tools were used:
The LLM found the malloc in the union merge_lines_to_draw and the int array leak, and implemented the bvh layer visualizer handle

Development OS / boundary conditions

Please describe what OS you developed and tested your additions on, and if any special dependencies are required:
MacOS Golden Bridge.

PR Checklist for contributing to McStas/McXtrace

For a coherent and useful contribution to McStas/McXtrace, please fill in relevant parts of the checklist:

My contribution includes patches to an existing component file
- I have used the mcdoc utility and rendered a reasonable documentation page for the component (please attach as screenshot in comments!)
- I have ensured that basic use of the component is OK (e.g. an instrument using it compiles?)
- I have used the mctest utility to test one or more instruments making use of the component (please attach mcviewtest report as screenshot in comments)
- I have used the mccode-clangformat tool to apply the standard McCode component indentation scheme
- I have used the mcrun --c-lint "linter" and followed advice to remove most / all warnings that are raised
My work touches / adds to the runtime lib code (.c,.h etc in multiple locations
- I am have added reasoning and documentation for the change below
- I am attaching test output in the comments

…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]>

Union mcdisplay: fix merge_lines_to_draw ownership, leaks and quadratic cost

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]>

Union_mesh: draw BVH boxes in mcdisplay for large meshes

Lomholy · 2026-10-23T19:33:06Z

mcviewtest on Union_mesh:

mcdoc on Union_mesh:

Lomholy · 2026-10-23T19:54:15Z

@willend and @mads-bertelsen sorry for the PR follow up so soon. I know how this is pushing you guys, but no rush regarding this PR, I just made it to flag the issue as well as a possible solution.

mads-bertelsen · 2026-10-23T20:22:47Z

@Lomholy no worries, it’s very good! Will have a look as soon as possible. Doubt the preceding mesh solution was better in this regard, so it’s still progress

mads-bertelsen · 2026-10-23T20:27:16Z


 struct rotation_struct rotation; // Not used, is the x y and z rotation angles.
 int visualization_on; // If visualization_on is true, the volume will be drawn in mcdisplay, otherwise not
+int mesh_display_layer; // mesh only: BVH layer whose boxes mcdisplay draws, 0 to draw the triangle edges


This is specific to mesh and should be in the mesh storage structure, now that is the mesh library struct, but it will need to be a struct of this value and the library structure

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]>

Union_mesh: keep the mcdisplay BVH layer in a mesh_storage struct

willend · 2026-10-24T14:42:07Z

I do know that Union_master / functions called from there draws the analysed geometry as single line-segments, but I am convinced speed of visualisation / transfer to the .py utils would likely become much quicker the moment we sent polygon's directly.

(And yes I know it is not a trivial rewrite to do, yet I think it is worth to have on the wish-list for some time in the future...)

willend · 2026-10-24T17:50:11Z

@Lomholy and @mads-bertelsen I couldn't help letting Claude have a go at a polygon-based Union_master display:

https://github.com/mccode-dev/McCode/pull/2898/changes is based on current main
Union: polyhedron display (#2898) combined with Union_mesh BVH display (#2893) #2899 is my changes stacked on this PR branch

willend · 2026-10-24T17:52:54Z

The default follows whatever --trace(=1 for lines or =2 for solids/polygonal) is on the commandline / from the given tool. Union_master also got a switch to force running with lines.

willend · 2026-10-27T19:38:21Z

@mads-bertelsen as we discussed today I prototyped a bit more on the visualisation-options for meshes / 'surface'-primitives in mcdisplay-webgl (and -classic). New work incoming in via #2915.

I aim to rebase the related Union-surface PR once the line-toggle PR tests are run to green and merged.

mads-bertelsen

The underlying problem seems to be that reporting all these line segments through mcdisplay for huge systems is not a good idea in the first place, and @willend 's experiments with passing polyhedra instead seems like a viable option, that can easily plot the dragon.stl.

That said the PR here is valuable for several reasons, it adds the capability to see the bounding boxes which is a good fallback in viewers where the polyhedra is not supported and it fixes some bugs in code that will still be used.

The Union_mesh component needs a quick clang-format pass, then we can merge this and build upon it with a polyhedra solution.

Lomholy · 2026-10-28T11:15:02Z

@mads-bertelsen Alright, sounds good! I have just performed the clangformatting so I suppose this can be merged whenever the tests go through 😄

p_mesh was renamed to p_mesh_storage (with the mesh inside) in 12058c4, merged to main via mccode-dev#2893; the main merge into this branch left the polyhedron code pointing at the old member. Co-Authored-By: Claude Opus 5.5 <[email protected]>

lomholy_agentic and others added 4 commits October 23, 2026 20:36

Merge pull request #18 from lomholy-agentic/union-mcdisplay-merge-fix

2e18fa0

Union mcdisplay: fix merge_lines_to_draw ownership, leaks and quadratic cost

Merge pull request #19 from lomholy-agentic/union-mesh-bvh-display

2e96b71

Union_mesh: draw BVH boxes in mcdisplay for large meshes

mads-bertelsen reviewed Oct 23, 2026

View reviewed changes

lomholy_agentic and others added 2 commits October 24, 2026 12:09

Merge pull request #20 from lomholy-agentic/union-mesh-storage

5a09eeb

Union_mesh: keep the mcdisplay BVH layer in a mesh_storage struct

willend mentioned this pull request Oct 24, 2026

Union: polyhedron display (#2898) combined with Union_mesh BVH display (#2893) #2899

Closed

willend added 2 commits October 26, 2026 19:14

Merge branch 'main' into union_mcdisplay_fix

2876ad8

Merge branch 'main' into union_mcdisplay_fix

4f90ae9

mads-bertelsen approved these changes Oct 28, 2026

View reviewed changes

Apply clangformat to union mesh component

1fb7781

Lomholy marked this pull request as ready for review October 28, 2026 11:15

mads-bertelsen merged commit 95bccfd into mccode-dev:main Oct 28, 2026
18 checks passed

Navigation Menu

Repository navigation

Union_mesh mcdisplay slow with large meshes - optimize merge_lines_to_draw and add option for drawing coarse meshes - #2893

Union_mesh mcdisplay slow with large meshes - optimize merge_lines_to_draw and add option for drawing coarse meshes#2893
mads-bertelsen merged 9 commits into
mccode-dev:mainfrom
Lomholy:union_mcdisplay_fix

Lomholy commented Oct 23, 2026 •

edited

Loading

Uh oh!

Lomholy commented Oct 23, 2026 •

edited

Loading

Uh oh!

Lomholy commented Oct 23, 2026

Uh oh!

mads-bertelsen commented Oct 23, 2026

Uh oh!

mads-bertelsen Oct 23, 2026

Uh oh!

willend commented Oct 24, 2026

Uh oh!

willend commented Oct 24, 2026 •

edited

Loading

Uh oh!

willend commented Oct 24, 2026

Uh oh!

willend commented Oct 27, 2026 •

edited

Loading

Uh oh!

mads-bertelsen left a comment

Uh oh!

Lomholy commented Oct 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Lomholy commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Free-form text area

Declaration of use of AI-tools

Development OS / boundary conditions

PR Checklist for contributing to McStas/McXtrace

For a coherent and useful contribution to McStas/McXtrace, please fill in relevant parts of the checklist:

My contribution includes patches to an existing component file

My work touches / adds to the runtime lib code (.c,.h etc in multiple locations

Uh oh!

Lomholy commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lomholy commented Oct 23, 2026

Uh oh!

mads-bertelsen commented Oct 23, 2026

Uh oh!

mads-bertelsen Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

willend commented Oct 24, 2026

Uh oh!

willend commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

willend commented Oct 24, 2026

Uh oh!

willend commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mads-bertelsen left a comment

Choose a reason for hiding this comment

Uh oh!

Lomholy commented Oct 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Lomholy commented Oct 23, 2026 •

edited

Loading

Lomholy commented Oct 23, 2026 •

edited

Loading

willend commented Oct 24, 2026 •

edited

Loading

willend commented Oct 27, 2026 •

edited

Loading