Skip to content

Adds support for variable order mfem nurbs curve meshes#1821

Open
kennyweiss wants to merge 17 commits intodevelopfrom
feature/kweiss/arbitrary-order-mfem-nurbs
Open

Adds support for variable order mfem nurbs curve meshes#1821
kennyweiss wants to merge 17 commits intodevelopfrom
feature/kweiss/arbitrary-order-mfem-nurbs

Conversation

@kennyweiss
Copy link
Copy Markdown
Member

Summary

  • This PR adds support for reading variable order mfem NURBS curve meshes in quest::MFEMReader.
    • These are often provided using the "patches" clause in the mfem format
    • This feature was added to mfem after its latest release mfem@4.9 (and we currently depend on mfem@4.9), so we maintain backwards compatibility. See: Nurbs variable 1d dev mfem/mfem#5167
    • I've also added an option to the 2D winding number application to output a degree-elevated version of the mfem file so it can be opened by downstream applications that have not yet updated to the new format. In particular, we can open these "elevated" files in VisIt.
  • I've updated the svg2contours python script to optionally output mfem meshes using variable orders (containing linear, quadratic and cubic polynomial elements as well as rational quadratics for the elliptical arcs. Previously, we were converting all input curves to rational cubics
  • I've update the quest_step_file application to optionally output one mfem mesh per surface patch containing its parametric geometry and trimming curves, and to optionally output a json file containing stats about each patch
    • Since we might want to skip patches that are trivially trimmed (i.e. whose trimming curves are aligned with the rectangular patch boundaries), I added an option to test for this (NURBSPatch::isTriviallyTrimmed() which uses the new NURBSCurve::isLinear()) and skip outputting patches for these files

Misc:

  • I had a python virtual environment (.venv) in my source directory, and the yapf styler tried to add all its python files. I updated the CMake logic for the code checks to (a) only operate on checked in files (via git ls-files) and to also explicitly skip .venv folders

@kennyweiss kennyweiss self-assigned this Mar 17, 2026
@kennyweiss kennyweiss added Quest Issues related to Axom's 'quest' component Primal Issues related to Axom's 'primal component Build system Issues related to Axom's build system labels Mar 17, 2026
@kennyweiss kennyweiss added this to the FY26 Summer release milestone Mar 17, 2026
* \brief Predicate to check if the patch is "trivially trimmed" in parameter space.
*
* A patch is considered trivially trimmed if either:
* - it has no trimming curves, or
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't know if I agree with the idea that "no trimming curves" should be treated the same way as "has trimming curves which don't really do anything." In particular, I can imagine a use case where you'd want to call

for(auto & patch : patches)
    if(patch.isTriviallyTrimmed())
        patch.makeTriviallyTrimmed();

and with this definition, you'd end up with patches containing potentially two very different kinds of surfaces, some which are the same as untrimmed, and some which are totally invisible.

I think this next one is potentially more controversial, but I also think that if the internal flag for trimmed-ness m_isTrimmed is false, this should probably return false too. In general, I think the semantics of isTriviallyTrimmed() should really mirror makeTriviallyTrimmed(), and the latter means "mark the patch as trimmed, and add four curves around the patch boundaries.

Copy link
Copy Markdown
Member Author

@kennyweiss kennyweiss Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So

  • if(!isTrimmed()) return false;
  • if(numCurves != 4) return false;

That could work

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so. Do you feel like that's intuitive enough, to have untrimmed patches return false if isTriviallyTrimmed is called on them?

{
const auto& curves = patch.getTrimmingCurves();
const int numCurves = curves.size();
const bool is_trivially_trimmed = patch.isTriviallyTrimmed();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to my above point about isTriviallyTrimmed(), if the goal here is to track all of the patches which have "boring" trimming geometry so they can be skipped during plotting, I'm not sure if "trivially trimmed" is the best way to describe it. If I understand correctly, what you're looking for is more similar to

const bool is_boring_to_plot = !patch.isTrimmed() || patch.isTotallyInvisible() || patch.isTriviallyTrimmed();

where the currently undefined isTotallyInvisible() returns true if the patch is marked as trimmed, but has no trimming curves. I think it's reasonable to have one simple predicate for a patch that checks all three of these conditions, but I don't know what you would call it. What do you think?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense -- should I add an isTotallyInvisible() function?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that would be useful. Maybe Totally is a little colloquial for a function name, but even isVisible or isInvisible without any arguments could work, like we talked about earlier.

… as mfem file

Uses new nurbs patch format, which was added to mfem after the 4.9 release.
This is a NURBS analogue to a function in the BezierCurve class.
This returns true if the patch has not trimming curves, or has four
trimming curves that form a loop around the patch boundary in parametric space.
…ample

This lists statistics like the curve orders and min/max parametric space bounds per patch.
When reading in the STEP file, we remove the periodicity, but users
might want to know if the patch was originally periodic.
…ormat

This lets us avoid degree elevation of all curves to degree 3.
…mfem mesh

This is in support of downstream applications (such as VisIt) that do not yet support
the variable-order mfem NURBS meshes (or patch-based mfem NURBS meshes)
which became available after the mfem@4.9 release.
* Only use checked in files (when using git)
* Don't try to lint files from a .venv virtual environment
I think it is due to a  `#define near` within the msvc headers.
@kennyweiss kennyweiss force-pushed the feature/kweiss/arbitrary-order-mfem-nurbs branch from 36fda1f to 71a2dba Compare March 24, 2026 03:16
Copy link
Copy Markdown
Contributor

@Arlie-Capps Arlie-Capps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kenny, thanks for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build system Issues related to Axom's build system Primal Issues related to Axom's 'primal component Quest Issues related to Axom's 'quest' component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants