Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement loading of meshes from MSH files #15

Merged
merged 15 commits into from
Dec 20, 2022

Conversation

w1th0utnam3
Copy link
Member

@w1th0utnam3 w1th0utnam3 commented Dec 29, 2021

This implements loading of Fenris meshes from MSH files/byte slices using the mshio crate. Only Gmsh MSH file format 4.1 is currently supported.

The io::msh module provides the functions load_msh_from_file and load_msh_from_bytes. The specified Connectivity type determines which element blocks are actually extracted from the loaded MSH file. All element types that implement the MshConnectivity trait are supported. Currently those are:

  • Hex8Connectivity
  • Hex27Connectivity
  • Quad4d2Connectivity
  • Quad9d2Connectivity
  • Tet4Connectivity
  • Tet10Connectivity
  • Tri3d2Connectivity
  • Tri3d3Connectivity
  • Tri6d2Connectivity

Tri6d3Connectivity, Quad4d3Connectivity and Quad9d3Connectivity are currently not supported because they don't implement the ElementConnectivity trait.

@w1th0utnam3 w1th0utnam3 force-pushed the msh_import branch 2 times, most recently from 09bff9c to fa45d81 Compare October 12, 2022 08:42
@w1th0utnam3 w1th0utnam3 force-pushed the msh_import branch 2 times, most recently from 5261245 to 983bd72 Compare November 10, 2022 15:51
Copy link
Member

@Andlon Andlon left a comment

Choose a reason for hiding this comment

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

Looks good to me! I left one comment about some commented out stuff; unsure if we should remove this before merge or keep it in, maybe with some added TODO statement?

More generally, I'm also unsure about the higher order elements, because currently the higher-order elements in fenris assume an affine reference -> physical mapping... But maybe we can merge and create an issue for fixing this in the future. I believe the fix is to support fully isoparametric elements and import directly into these, then having some kind of assume_affine() function to turn them into elements with affine transformations. Not sure!

src/io/msh.rs Show resolved Hide resolved
{
return Err(eyre!("node block entity does not have the right dimension for this mesh"));
}
*/
Copy link
Member

Choose a reason for hiding this comment

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

Is this still relevant? Should it be left in but commented? It's not clear to me why it's commented out, which probably means it should either be removed or fixed? (Or replaced with a TODO or something)

@w1th0utnam3 w1th0utnam3 force-pushed the msh_import branch 5 times, most recently from 6e26900 to 6642394 Compare November 16, 2022 13:19
Copy link
Member

@Andlon Andlon left a comment

Choose a reason for hiding this comment

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

Basically ready to merge, but it looks like parts of #49 sneaked into this PR? Since that one is still pending some basic tests I'd prefer to leave this out of this one.

weights: self.weights,
data: data,
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This appears to be unrelated to MSH, and seems to be the same functionality as in #49 ? Also applies to code below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, that was not intended.

Copy link
Member

@Andlon Andlon left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@Andlon Andlon merged commit 672c650 into InteractiveComputerGraphics:master Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants