Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Fix mesh_xsection #4

Merged
merged 10 commits into from
May 26, 2017
Merged

Fix mesh_xsection #4

merged 10 commits into from
May 26, 2017

Conversation

algrs
Copy link
Contributor

@algrs algrs commented May 19, 2017

Plane.mesh_xsection was a bit of a mess, and in particular it sometimes returned a "Polyline" that was actually a disordered set of lines, not an end to end connected ring. And it relied on a hack passing edges to bodylabs.mesh.topology.connectivit.remove_redundant_verts instead of faces and relying on a bug in it that fails to verify that the "faces" are actually Nx3. So. This is closer to a correct algorithm. And now with some real tests. And wrote it as Plane.mesh_xsections which always returns a list of Polylines, one for each connected component. Then wrote a Plane.mesh_xsection that mangles that result back into the jammed together single Polyline that it used to return.

@jbwhite can you test that this still works with measurements?

@eerac just FYI.

@algrs algrs requested review from eerac and jbwhite May 19, 2017 19:39
@algrs
Copy link
Contributor Author

algrs commented May 22, 2017

@jbwhite Hold off on reviewing this for a moment -- it may not work with real meshes.

@jbwhite
Copy link
Contributor

jbwhite commented May 22, 2017

@algrs OK -- did try to run one measurement using it, and it did not complete (did not troubleshoot why).

@algrs
Copy link
Contributor Author

algrs commented May 22, 2017

Yep, there was a problem that was causing it to never terminate on a large and complex mesh. Rewrote a bunch more of it. Give it a read; most of it should make some sense at this point.

@jbwhite
Copy link
Contributor

jbwhite commented May 22, 2017

@algrs When testing the values returned for this vs master, I'm seeing the same values for vertices but in a different order (on this branch they appear sorted by the first column).

I've attached an output of results -- used pdb.set_trace() to inspect the values:

mesh_xsection_results.txt

@algrs
Copy link
Contributor Author

algrs commented May 22, 2017 via email

@algrs
Copy link
Contributor Author

algrs commented May 22, 2017 via email

# This works because eulerPath mutates the graph as it goes
path = eulerPath(E)
if path is None:
raise ValueError("mesh slice has too many odd degree edges; can't find a path along the edge")
Copy link
Contributor

@jbwhite jbwhite May 22, 2017

Choose a reason for hiding this comment

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

Raising an error here is a problem as much of the calling code expects that sometimes the result will fail to find a path.

The cross_section run function in measurements builds a collection of candidate_polylines and expects to get results where result.v is None:

        for plane in self.planes:
            raw_xs = plane.mesh_xsection(self.trimmed_mesh)
            if raw_xs.v is None:
                continue
            convex_xs = convexify.convexify_planar_curve(raw_xs, flatten=True)
            self.candidate_polylines.append(convex_xs)

Copy link
Contributor

Choose a reason for hiding this comment

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

Will need a different way to break out of the loop too I think -- when I take out the raise this while loop just keeps running

# Since we're willing to take away one connected component per call,
# we skip this check.
# if len(odd) > 3:
# return None
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's relevant to the underlying algorithm; I think leaving it in there makes sense, explaining why we diverge from the algorithm named.

else:
return "<open Polyline with {} verts>".format(len(self))
else:
return "<Polyline with no verts>"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@algrs algrs merged commit de49d87 into master May 26, 2017
@algrs algrs deleted the 123_mesh_xsection branch May 26, 2017 16:47
paulmelnikow added a commit that referenced this pull request Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants