Skip to content

Conversation

@krober10nd
Copy link
Collaborator

fixing bug from #166
@WPringle
Copy link
Collaborator

I also encountered this strange ordering before but I remember depending on the clockwise/anticlockwise choice would produce different results. Would be good to find out the source of the error instead of the ad-hoc adjustment at the end.

@krober10nd
Copy link
Collaborator Author

It isn't ad hoc but it does suggest other changes. The edges are ordered in a way that creates this problem

Perhaps we should get rid of this CW and CCW option and instead just make people switch the ordering of the inputs. It's not clear what CW and CCW always mean for complex polygons and most people (including me) simply guess.

@krober10nd
Copy link
Collaborator Author

Perhaps the default should be 1 and leave it like that.

@WPringle
Copy link
Collaborator

Right so tell people they have to enter it in Clockwise ordering? Which is 1 again?
I still would like to see where the wrong ordering is coming from and I'm worried that this doesn't always apply. It must be right most of the time because doesn't often produce this error.

@krober10nd
Copy link
Collaborator Author

Right so tell people they have to enter it in Clockwise ordering? Which is 1 again?

Yes something like. Just get rid of this option in the API.

In the file utilities/extract_boundary.m I describe the algorithm for the "traversal". We can have a closer look at it, it's been years since I wrote it.

@krober10nd
Copy link
Collaborator Author

Running the JBAY example and picked an arbitrary segment just switching the order of the input indices keeping the order set to 1.

Screen Shot 2020-12-23 at 3 22 19 PM

Screen Shot 2020-12-23 at 3 22 33 PM

@krober10nd
Copy link
Collaborator Author

I think I've found the origin of the problem. Line 66-67 of utilities/extract_boundary adds the entire selected edge to the polygon list ignoring the v_start and v_next while all other segments consider this.

v_next must appear second in the list for the first edge.
@krober10nd
Copy link
Collaborator Author

Okay put in a fix. If v_next doesn't appear as the last entry of the first edge selected, the edge is flipped so that is the case.

@WPringle
Copy link
Collaborator

Ok cool so it definitely works for normal open boundary selection as well as small river like segments?

if segment is one edge long (2 vertices).
@krober10nd
Copy link
Collaborator Author

I've tested it with a one edge segment with order=0 and order=1 on two different meshes and they all worked. I've also tested segments for length 3 vertices and on larger domains as well and the original problem did not crop up.

@krober10nd krober10nd merged commit 84cbcc1 into Projection Dec 23, 2020
@krober10nd krober10nd deleted the boundary_draw branch December 23, 2020 21:26
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.

3 participants