Hello! Andreas, thank you for your contribution! It's good to see some work going into commons-geometry since I've had to step away recently. Gilles is right in that we didn't include the hull module in the 1.0 release because I felt like it still needed work. What I'd like to do is remove that module completely and pull the convex hull generation directly into the euclidean module and hide the implementation algorithm details (as discussed in GEOMETRY-144). I've replied to your comment on that Jira issue and I hope to take a detailed look at your code over this weekend, if not before. If you're interested in seeing the API I was aiming for when working on this, my unfinished code is languishing on Github [1].
Regards, Matt J [1] https://github.com/darkma773r/commons-geometry/blob/quickhull/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/threed/ConvexHull3D.java On Mon, Jan 30, 2023 at 2:38 AM Andreas Goss <andreas.goss1...@gmail.com> wrote: > Thanks for the quick reply. I need to create an Jira account first for > that. I requested an account by writing to priv...@commons.apache.org > > Kind Regards > Andreas > > Gilles Sadowski <gillese...@gmail.com> schrieb am Mo. 30. Jan. 2023 um > 00:11: > > > Hello. > > > > Le dim. 29 janv. 2023 à 19:25, Andreas Goss > > <andreas.goss1...@gmail.com> a écrit : > > > > > > Hello, > > > > > > I want to contribute a solution for the open Jira task GEOMETRY-110 ( > > > > > > https://issues.apache.org/jira/projects/GEOMETRY/issues/GEOMETRY-110?filter=allopenissues > > ). > > > I tried my best to organize the code according to guidelines and at > least > > > the maven build was succesful. I created a pull-request for the github > > > clone. If someone could give me a code review and point out areas to > > > improve the code or errors i would be very thankful. > > > > Thank you for tackling this issue. > > It was part of a broader discussion about the API around the hull > > functionality. > > IIRC, the "commons-geometry-hull" module was not included in the first > > release of "Commons Geometry" because Matt Juntunen was convinced > > that a few enhancements were necessary. Hopefully, he'll chime in order > to > > make this recollection more precise. > > > > A general question is whether we want to expose (make "public") classes > > that implement the algorithm(s), such as "QuickHull3D". For example, we > > could perhaps have (untested and required comments missing...): > > ---CUT--- > > public interface ConvexHull3D extends ConvexHull<Vector3D> { > > > > public enum Generate { > > QUICK_HULL((c, p) -> new QuickHull3D(p).generate(c)); > > > > private final BiFunction<Collection<Vector3D>, > > DoubleEquivalence, ConvexHull3D> generator; > > > > private HullGenerator3D(BiFunction<Collection<Vector3D>, > > DoubleEquivalence, ConvexHull3D> generator) { > > this.generator = generator; > > } > > > > public ConvexHull3D from(Collection<Vector3D> points, > > DoubleEquivalence equivalence) { > > return generator.apply(points, equivalence)); > > } > > } > > > > public Collection<? extends ConvexPolygon3D> getFacets(); > > } > > > > private class QuickHull3D { > > private final DoubleEquivalence precision; > > > > QuickHull3D(DoubleEquivalence precision) { > > this.precision = precision; > > } > > > > ConvexHull3D generate(Collection<Vector3D> points) { > > // ... > > return new SimpleConvexHull3D(...); > > } > > } > > > > private class SimpleConvexHull3D implements ConvexHull3D { > > private final List<Vector3D> vertices; > > private final ConvexVolume region; > > private Collection<ConvexPolygon3D> facets; > > > > SimpleConvexHull3D(...) { > > // ... > > } > > > > // ... > > } > > ---CUT--- > > > > For other concrete (basic) remarks about the code, we can certainly > > continue the conversation in comments on the JIRA report. > > > > Best regards, > > Gilles > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > >
