[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16844098#comment-16844098
]
Gilles commented on GEOMETRY-29:
bq. Is there a formatter
You could ask on the ML; more people who use an IDE...
I mostly rely on CheckStyle (but it seems that the brace style is not enforced).
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16844021#comment-16844021
]
Sven Rathgeber commented on GEOMETRY-29:
{quote}There were some additional nits: See commit
7a42a6f317b0de8ee63b0461e868532c8e025b4e
{quote}
[~erans] Is there a formatter or a report for this coding style ?
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16843973#comment-16843973
]
Gilles commented on GEOMETRY-29:
Note: I'd guess that it is possible to "merge" commits within GitHub. If so,
it would provide for a nicer history (on the Apache repository) if unimportant
commits are squashed together.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16843970#comment-16843970
]
Gilles commented on GEOMETRY-29:
Thanks.
There were some additional nits: See commit
7a42a6f317b0de8ee63b0461e868532c8e025b4e
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16843770#comment-16843770
]
Sven Rathgeber commented on GEOMETRY-29:
[~erans], I have added a commit to the PR.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16843729#comment-16843729
]
Gilles commented on GEOMETRY-29:
[~rathgeber], could you please fix all the errors reported by "CheckStyle"?
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16843580#comment-16843580
]
Matt Juntunen commented on GEOMETRY-29:
---
[~erans], can you please merge this PR? I'm going to need to use it very soon.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16814176#comment-16814176
]
Sven Rathgeber commented on GEOMETRY-29:
{quote}make origin computed instead of stored
make areCoplanar a private helper method in Plane (The reasoning behind this
is to try to keep the public API small and maintainable as a general rule and
only add methods if required. It's easier by far to add things later than to
remove them.)
{quote}
Done
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16813974#comment-16813974
]
Matt Juntunen commented on GEOMETRY-29:
---
{quote}A user guide would be welcome...
{quote}
Is this a blocker?
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16813965#comment-16813965
]
Gilles commented on GEOMETRY-29:
No. I'm looking from afar, and I mostly rely on you until I'll get a chance to
practically test some of this. ;-)
A user guide would be welcome...
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16813961#comment-16813961
]
Matt Juntunen commented on GEOMETRY-29:
---
Sounds good. [~rathgeber], could you make those last changes?
- make {{origin}} computed instead of stored
- make {{areCoplanar}} a private helper method in {{Plane}} (The reasoning
behind this is to try to keep the public API small and maintainable as a
general rule and only add methods if required. It's easier by far to add things
later than to remove them.)
[~erans], is there anything that might prevent you from merging this after the
above items are complete?
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16811330#comment-16811330
]
Gilles commented on GEOMETRY-29:
{quote}what do you think?
{quote}
I'd assume that if you wonder, it is better, at this point, to say "no" to both.
It's of course good to leave a note (code comment) and revisit later.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
> Labels: pull-request-available
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16809395#comment-16809395
]
Matt Juntunen commented on GEOMETRY-29:
---
I just realized that the PR isn't linked to this JIRA issue, I suspect since
the name is "Geometry 29" and not "GEOMETRY-29". I just added it as a link
manually. Hopefully the Apache gods will not be angered.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16809389#comment-16809389
]
Matt Juntunen commented on GEOMETRY-29:
---
This PR is looking good. [~erans], what do you think? I'd like your opinion on
two things specifically:
1. To store or not to store the computed {{origin}} property?
2. Creation of {{Vector3D#areCoplanar()}} method; should this be part of the
public API?
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16801399#comment-16801399
]
Matt Juntunen commented on GEOMETRY-29:
---
Thanks, [~rathgeber]! I added some comments on Github.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16800653#comment-16800653
]
Sven Rathgeber commented on GEOMETRY-29:
I have added a pull request:
[https://github.com/apache/commons-geometry/pull/31]
which currently tells my, that I decreased test coverage :( I will work on
that.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16791829#comment-16791829
]
Matt Juntunen commented on GEOMETRY-29:
---
{quote}I don't see the use cases for hashCode and equals for values based on
doubles.
{quote}
I agree that in many situations fuzzy comparison of doubles is preferred, but I
don't think that means we should not implement these methods. A situation where
I might rely on these methods is when reuniting split subhyperplanes in order
to generate the boundary of a BSP tree. Each subhyperplane has a hyperplane
that embeds it, and there may be many subhyperplanes in the tree with exactly
equal hyperplanes. In this case, I would actually use the hyperplanes as keys
in a map in order to find all subhyperplanes that lie in the same hyperplane.
In regard to fuzzy comparisons, the {{Vector?D}} classes allow this
functionality through an overloaded {{equals}} method that takes a
{{DoublePrecisionContext}}. We have not implemented this in any of the
hyperplane classes since it hasn't seemed to be needed at this point. It's also
a bit complicated by the fact that the hyperplane classes include their own
{{DoublePrecisionContext}} so there isn't an immediately obvious method
overload opportunity, not to mention the issue of which hyperplane's precision
context to use when performing the comparison.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16791700#comment-16791700
]
Sven Rathgeber commented on GEOMETRY-29:
This code is generated by eclipse (Version: 2018-12 (4.10.0)) and following the
recommendations of Joshua Bloch (Effective Java, 3rd Edition).
I don't see the use cases for hashCode and equals for values based on doubles.
I would not put a Vector3D, Line, Plane etc. as key in a Map.
When I compare computed double values, I always use a delta/doublePrecision
approach to avoid the trouble with rounding differences.
WDYT
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16791651#comment-16791651
]
Matt Juntunen commented on GEOMETRY-29:
---
{quote}hmm, what do you picture here ? double comparison ...,
precisionContext
{quote}
I'm picturing following the same approach as the other refactored hyperplane
classes, especially
[o.a.c.g.euclidean.twod.Line|https://github.com/apache/commons-geometry/blob/master/commons-geometry-euclidean/src/main/java/org/apache/commons/geometry/euclidean/twod/Line.java#L338].
Basically, the standard {{equals}} and {{hashCode}} methods perform exact
comparisons in all cases, which goes for other objects in the library as well.
Fuzzy comparisons are left for other methods.
bq. This one, does not make sense to me:
I'm not sure what you mean? Where did this code come from?
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16791523#comment-16791523
]
Sven Rathgeber commented on GEOMETRY-29:
{quote}add {{equals}}, {{hashCode}} methods.
{quote}
hmm, what do you picture here ? double comparison ..., precisionContext
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16790499#comment-16790499
]
Matt Juntunen commented on GEOMETRY-29:
---
Great! No, I haven't done anything on this yet, so feel free. I'm still working
on GEOMETRY-32 now.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16790377#comment-16790377
]
Sven Rathgeber commented on GEOMETRY-29:
{quote}feel free to work on this if you have the time and feel so inclined.
I'll be working on the BSP tree stuff so I won't be stepping on your toes.
{quote}
[~mattjuntunen] : did you start on this ? This week I have some time and could
do it.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16771049#comment-16771049
]
Matt Juntunen commented on GEOMETRY-29:
---
bq. Should rather be removed.
I agree. It's part of current {{Hyperplane}} interface but I'll see if I can
remove it in GEOMETRY-32.
[~rathgeber], feel free to work on this if you have the time and feel so
inclined. I'll be working on the BSP tree stuff so I won't be stepping on your
toes.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16770852#comment-16770852
]
Gilles commented on GEOMETRY-29:
{quote}
// return this since now immutable
Plane copySelf();
{quote}
Should rather be removed.
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16770787#comment-16770787
]
Sven Rathgeber commented on GEOMETRY-29:
That looks good and consistent to me.
{quote}I just had another thought in regard to the {{getOffset}} method: if we
interpret the method in a general sense as "give me the offset closest to zero
between two geometric elements" (ie, "what's the smallest distance between
them"), then the return value between two non-parallel lines in 2D or a
non-parallel line and plane in 3D would simply be zero, since the elements
intersect at some point. The value would also be zero for parallel elements
that coincide, but callers could use other methods if they need to distinguish
between these two states. This approach feels the cleanest and most consistent
to me so I think we should go that route.{quote}
Agreed !
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (GEOMETRY-29) Plane API cleanup
[
https://issues.apache.org/jira/browse/GEOMETRY-29?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16770729#comment-16770729
]
Matt Juntunen commented on GEOMETRY-29:
---
Here are some more details of what I'm picturing for the API, based on the
recently updated {{o.a.c.geometry.euclidean.oned.OrientedPoint}} and
{{o.a.c.geometry.euclidean.twod.Line}} classes:
{code:java}
public final class Plane implements ... {
// all member variables final
// no public ctor; all instance creation is through static factory methods
// remove reset(), setNormal(), and setFrame() methods
Vector3D getOrigin();
Vector3D getNormal();
Vector3D getOrigin();
Vector3D getU();
Vector3D getV();
Vector3D getW(); // alias for getNormal()
Vector3D project(Vector3D);
Line project(Line); // new method; 3D line projected onto plane
DoublePrecisionContext getPrecision();
// return this since now immutable
Plane copySelf();
// used to be named revertSelf(); the behavior should be the same but
// it should return a new Plane instead of modifying the instance
Plane reverse();
Vector2D toSubSpace(Vector3D) {... }
Vector3D toSpace(Vector2D) {... }
boolean contains(Vector3D) {... }
boolean contains(Line) {... } // 3D line
boolean contains(Plane) {... } // renamed from isSimilarTo()
double getOffset(Vector3D);
double getOffset(Line); // 3D line
double getOffset(Plane);
// renamed from getPointAt()
Vector3D pointAt(Vector2D, double) {... }
// these may be removed after GEOMETRY-24 but they should be retained for
now
Plane rotate(Vector3D, QuaternionRotation);
Plane translate(Vector3D);
Vector3D intersection(Line); // 3D line
Line intersection(Plane); // 3D line
SubPlane wholeHyperplane();
PolyhedronsSet wholeSpace();
boolean sameOrientationAs(Hyperplane);
int hashCode();
boolean equals(Object);
String toString();
// u = p1.vectorTo(p2); v = p1.vectorTo(p3); w = u x v
static Plane fromPoints(Vector3D p1, Vector3D p2, Vector3D p2);
// u = normal.orthogonal(); v = w x u
static Plane fromPointAndNormal(Vector3D pt, Vector3D normal);
// same as above but uses the origin as the point
static Plane fromNormal(Vector3D normal) {
return fromPointAndNormal(Vector3D.ZERO, normal);
}
// v = u.orthogonal(v); w = u x v
static Plane fromPointAndPlaneVectors(Vector3D pt, Vector3D u, Vector3D v);
// same as current implementation, although I think we can clean this up
considerably
static Vector3D intersection(Plane, Plane, Plane);
}
{code}
> Plane API cleanup
> -
>
> Key: GEOMETRY-29
> URL: https://issues.apache.org/jira/browse/GEOMETRY-29
> Project: Apache Commons Geometry
> Issue Type: Improvement
>Reporter: Matt Juntunen
>Priority: Major
>
> The following changes should be made to the
> {{o.a.c.g.euclidean.threed.Plane}} class:
> * make the class immutable
> * use well-named factory methods instead of constructor overloads
> * provide a factory method to create a plane with user-supplied {{u}} and
> {{v}} axes. The current implementation allows the normal to be provided but
> chooses its own planar axes (see {{setFrame}}).
> * add {{equals}}, {{hashCode}}, and {{toString}} methods.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
