Introduce TriaAccessor::combined_face_orientation() #12878
Conversation
| return this->face_orientation(face) + 4 * this->face_flip(face) + | ||
| 2 * this->face_rotation(face); |
There was a problem hiding this comment.
I am thinking about the following two points:
- Maybe we should change the type from
unsigned charto a struct/class built aroundunsigned char. This data structure would be the only place dealing with the shifts and would provide appropriate functions to get the right bits. - In MatrixFree,
face_orientationis flipped, isn't it? Maybe should we should do this as well here? @kronbichler
There was a problem hiding this comment.
Indeed, in MatrixFree the choice is such that the default case without any re-orientation or rotation is a plain 0, which simplifies life for the compiler to generate as good assembly code as possible for the most common case.
peterrum
changed the title
include/deal.II/grid/tria_accessor.h
Outdated
| @@ -899,6 +899,13 @@ class TriaAccessor : public TriaAccessorBase<structdim, dim, spacedim> | |||
| * @{ | |||
| */ | |||
|
|
|||
| /** | |||
| * Return a combination of face_orientation(), face_flip(), and | |||
| * face_rotation() for the given @p face. | |||
There was a problem hiding this comment.
Maybe say "Return an integer representation that uniquely encodes the orientation, flip, and rotation of a face."
| * face_rotation() for the given @p face. | ||
| */ | ||
| unsigned char | ||
| combined_face_orientation(const unsigned int face) const; |
There was a problem hiding this comment.
Yes, this is useful. I would actually prefer if that function simply forwarded to a member of ReferenceCell that then does the combining of the three flags. I think of the latter operation as a geometric primitive.
peterrum
force-pushed
the
combined_face_orientation
branch
from
08062ed
to
14975f5
Compare
|
@bangerth @kronbichler I would suggest to merge as it is now and we make the potential changes (char to struct, flip the orientation bit) in follow-up PRs. |
|
Sounds good to me. /rebuild |
As noted in https://github.com/dealii/dealii/pull/12859/files#r735246075.
Depends on #12859.