Validate shell orientations #1968
Conversation
|
Thank you for the pull request, @A-Walrus! And sorry for taking a few days to get to it. I had other things on my mind over the weekend 😄
I think checking that the boundaries are reverse should be the answer here. That requires the coordinate systems of all local path that refer to the same curve to match each other. Which I think is a reasonable requirement, exactly because we need to address cases like this. We already have the |
This solution almost works, but fails on edges that start and end at the same vertex.
A-Walrus
force-pushed
the
validate_orientations
branch
from
73e0cd8
to
714a6a0
Compare
There was a problem hiding this comment.
Thank you, @A-Walrus, looks good!
One thing to note is that the new validation check relies on GlobalEdge, which is on its way to being removed. There is already the new Curve, which fulfills the same purpose but is more flexible.
As part of my work on #1937, I've already amended the other validation checks to also use Curve, in preparation of removing GlobalEdge at a later point.
All of that is no reason not to merge this pull request. I'm just providing some context on how the code being changed here is currently developing.
| shell: &Shell, | ||
| errors: &mut Vec<ValidationError>, | ||
| ) { | ||
| let mut global_to_half: HashMap<ObjectId, Vec<_>> = HashMap::new(); |
There was a problem hiding this comment.
For some additional type safety, it should be possible to replace ObjectdId with HandleWrapper<GlobalEdge> here. This was not possible before, due to a horribly shameful workaround (:grin:), but that workaround was removed in #1951.
This is just a note, not a change request. I don't think it would make a difference here.
Addresses part of #1935.
Currently bugged, so marked as draft.
The problem is how to check whether two half-edges are reverse of one another:
Any help appreciated, thanks!