Hello, thank you very much! I am looking forward to your review comments since this is my first time trying to contribute to an open source project. I also noticed a few things with my code, that should be corrected. Some comments don’t make sense and there shouldn’t be any need for ordering the edges like i do right now in my current implementation. Also I think the code should then be refactored to fit better the desired API design. Also a discussion should be held if face merging is necessary.
Greetings Andreas Matt Juntunen <matt.a.juntu...@gmail.com> schrieb am Mi. 1. Feb. 2023 um 06:20: > 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 > > > > > > > > >
