Merged
Conversation
…r a edge is boundary or not : replaced check of tag & MG_BDY by ftag & MG_BDY
Algiane
reviewed
Dec 19, 2023
src/mmg3d/colver_3d.c
Outdated
Comment on lines
+396
to
+397
| if ((pxt->ftag[MMG5_ifar[i][0]] & MG_BDY) || (pxt->ftag[MMG5_ifar[i][1]] & MG_BDY)) { | ||
| *tag |= MG_BDY; |
Member
There was a problem hiding this comment.
- The
if ( pxt->tag[i] & MG_BDY )can be kept as it may allow to exit the loop faster; - You can assign both
pxt->tag[i]andMG_BDYtag using the bitewise OR operatorpxt->tag[i] | MG_BDY
Finally, the test must be something like that:
if ( pxt->tag[i] & MG_BDY ) {
*tag |= pxt->tag[i];
*filled =1;
return adj;
} else if ((pxt->ftag[MMG5_ifar[i][0]] & MG_BDY) || (pxt->ftag[MMG5_ifar[i][1]] & MG_BDY)) {
*tag |= (pxt->tag[i] | MG_BDY);
*filled = 1;
return adj;
}
src/mmg3d/colver_3d.c
Outdated
Comment on lines
+449
to
+450
| if ((pxt->ftag[MMG5_ifar[ia][0]] & MG_BDY) || (pxt->ftag[MMG5_ifar[ia][1]] & MG_BDY)) { | ||
| *tag |= MG_BDY; |
Merged
…nditions to check wether an edge has to be considered boundary
Member
|
Thanks for this bugfix. |
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
2 participants
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.
This update fixes the triggering of a warning relative to the check of the number of boundary faces in the shell of a manifold edge.
Issue description
Collapses would sometimes merge two boundary manifold edges (that would each have 2 boundary faces in their shells) into one edge with 4 faces in the shell.
It happens when a non-boundary triangle has 3 boundary edges and we collapse one of the boundary edge (the two other edges being merged together).
(See attached picture were the edge
p-qis collapsed, the 3 colored edges are boundary edges and the trianglep-p-qis an internal triangle).To avoid this situation, when collapsing an edge, we travel the shell of the edge and we check the presence of an "internal triangle with 3 boundary edges" situation. It requires to be able to identify if an edge is a boundary edge.
The bug solved by this PR is due to the non-consistency of boundary tags between faces and edges.
Tag management in Mmg
Currently, the Mmg convention seems to be the following one:
MG_BDY+another tag if it is related to a given feature (ridge, ref edge, nosurf...);MG_BDYor not marked (0 tag) if it is a regular surface edge (regular surface edges without tags are created by the pattern splittings, see here.MG_BDYtag or not (it is due to the fact that it is expensive to update the edge tag informations for each edge of a tetra when a xtetra is added to the tetra during a collapse, thus, only edges of the new boundary face are updated).Note that it seems that we have few tests that seems erroneous regarding this convention:
Resolution
As the
MG_BDYtag is not always present along regular boundary edges, instead of checking forxt->tag[i] & MG_BDY, we now check the edge tag from a boundary face (we testxt->ftag & MG_BDYfor both faces sharing edge i): if the edge has no tag at all, it is a regular edge so we now that the edge is exactlyMG_BDY, if it has a non-nul tag, we recover the tag stored in the xtetra .