Hello.
Le lun. 19 août 2019 à 05:14, Matt Juntunen
<matt.juntu...@hotmail.com> a écrit :
>
> Hi Gilles,
>
> I did intend on adding more convenience methods for generating standard
> shapes but I definitely think we should abstract it like you're suggesting.
> Your design got me thinking and I believe we could take the idea of a
> FacetGenerator further and make it represent anything at all that can produce
> a sequence of facets,
+1
> be it a shape generator, a mesh, a 3D model file, a database table, a
> RegionBSPTree3D, etc. We could make full use of the streams API as well.
+1
> The main interface would look like this:
>
> public interface FacetSource {
> Stream<ConvexSubPlane> facetStream();
> }
IIUC the code in "RegionBSPTree3D", a facet is a "List<Vector3D>" (thus
not a "ConvexSubPlane". What am I missing?
>
> This would essentially perform the same task as your FacetGenerator, but it
> would allow facets to be streamed in one at a time instead of loaded into
> memory up front for each use. This would allow us to have code like this:
>
> RegionBSPTree3D tree = RegionBSPTree3D.builder(precision)
> .add(new RectangleFacetSource(p1, p2))
> .build();
>
> -- or --
>
> RegionBSPTree3D tree = RegionBSPTree3D.empty();
> new RectangleFacetSource(p1, p2).facetStream()
> .map(f -> f.transform(transform))
> .filter(f -> f.getPlane().getNormal().dot(vec) > 0)
> .forEach(tree::insert);
>
> -- or even --
>
> RegionBSPTree3D tree;
> try (Stream<ConvexSubPlane> stream = new
> OBJFacetSource("model.obj").facetStream()) {
> tree = stream.map(f -> f.transform(transform))
> .collect(FacetCollectors.toTree());
> }
>
> What do you think of this slightly modified approach?
Looks neat.
Regards,
Gilles
>
> -Matt
>
> ________________________________
> From: Gilles Sadowski <gillese...@gmail.com>
> Sent: Saturday, August 17, 2019 6:00 PM
> To: Commons Developers List <dev@commons.apache.org>
> Subject: Re: [geometry] GEOMTRY-32 Feedback Requested
>
> Hello.
>
> Le sam. 17 août 2019 à 04:58, Matt Juntunen
> <matt.juntu...@hotmail.com> a écrit :
> >
> > Hi Gilles,
> >
> > 1. I just rebased from master so hopefully that fixed the repo weirdness.
>
> It's fixed indeed.
>
> > 2. The best place to start working with the new code is probably the
> > RegionBSPTreeXD classes.
>
> Thanks for the pointer.
>
> > These are the replacements for the old IntervalsSet, PolygonsSet, and
> > PolyhedronsSet classes. I'm including a short example at the end of this
> > email that shows some of the functionality.
> > 3. Anything could be stored in the AttributeBSPTree nodes, but there
> > isn't specific functionality for computing the surface of portions of the
> > tree, since that requires the concept of inside and outside nodes, which is
> > present in the RegionBSPTree subclasses but not here. Is that what you were
> > asking?
>
> I think that what I called surface element is the equivalent of "facet".
>
> > 4. I was picturing adding both an examples module and a benchmark module
> > a littler later, along with a user guide.
>
> Great.
>
> >
> > -Matt
> >
> > DoublePrecisionContext precision = new EpsilonDoublePrecisionContext(1e-10);
> >
> > // Create a pyramid region with its base on the xy plane and its apex
> > // along the positive z axis. The base is 2 units to a side and the pyramid
> > // is 2 units high.
> > Vector3D[] vertices = {
> > Vector3D.of(1, 1, 0),
> > Vector3D.of(1, -1, 0),
> > Vector3D.of(-1, -1, 0),
> > Vector3D.of(-1, 1, 0),
> > Vector3D.of(0, 0, 2)
> > };
> >
> > RegionBSPTree3D region = RegionBSPTree3D.builder(precision)
> > .withVertexList(vertices)
> >
> > // add the faces; these could alternatively be given as a 2d array
> > .addIndexedFacet(0, 1, 2, 3)
> > .addIndexedFacet(0, 4, 1)
> > .addIndexedFacet(1, 4, 2)
> > .addIndexedFacet(2, 4, 3)
> > .addIndexedFacet(3, 4, 0)
> > .build();
> >
> > System.out.println("# Pyramid" +
> > "\nbarycenter= " + region.getBarycenter() +
> > "\nvolume= " + region.getSize() +
> > "\nsurface area= " + region.getBoundarySize());
> >
> > // Create a unit box centered at the origin
> > RegionBSPTree3D box = RegionBSPTree3D.builder(precision)
> > .addCenteredCube(Vector3D.ZERO, 1)
> > .build();
>
> I've had a quick look at "RegionBSPTree3D" as you suggested.
>
> First questions (from a newbie POV):
> * Do you intend to add more convenience methods to the
> "Builder" (such as "addCube" and "addCenteredCube")? If not, shouldn't
> the class be extensible (currently it is "final")? If yes, where do we stop
> ("addSphere", "addPyramid", "addTorus", ...)?
> * Wouldn't it be cleaner to separate "core" operations ("addFacet",
> "addIndexedFacets", ...) from convenience methods that use them
> ('addCube", ...)?
> * Then couldn't we define a more generic way to specify how to construct
> all sorts of shapes? E.g.:
> ---CUT---
> public class RegionBSPTree3D {
> // ...
> public final class Builder {
> public Builder addFacets(List<List<Vector3D>> facets) {
> for (List<Vector3D> f : facets) {
> addFacet(f);
> }
> }
> public Builder add(FacetGenerator gen) {
> return addFacets(gen.build());
> }
> // ...
> }
> // ...
> }
>
> And e.g.
> ---CUT---
> public classs RectangleGenerator implements FacetGenerator {
> /*
> * @param a first corner point in the rectangular prism (opposite
> of {@code b})
> * @param b second corner point in the rectangular prism (opposite
> of {@code a})
> */
> public RectangleGenerator(Vector3D a, Vector3D b) {
> // ...
> }
>
> @Override
> List<List<Vector3D>> build() {
> // ...
> }
> }
> ---CUT---
>
> WDYT?
>
> Various nit-picks:
>
> * Please put a space around operators:
> ---CUT---
> for (int i=0; i<vertexIndices.length; ++i)
> ---CUT---
> should be
> ---CUT---
> for (int i = 0; i < vertexIndices.length; i++)
> ---CUT---
>
> * Use "final" local variables wherever the reference won't be assigned a
> new value.
>
> * Use one condition per line:
> ---CUT---
> if (precision.eq(minX, maxX) || precision.eq(minY, maxY) ||
> precision.eq(minZ, maxZ))
> ---CUT---
> should be
> ---CUT---
> if (precision.eq(minX, maxX) ||
> precision.eq(minY, maxY) ||
> precision.eq(minZ, maxZ))
> ---CUT---
>
>
> Best,
> Gilles
>
> > System.out.println("\n# Box" +
> > "\nbarycenter= " + box.getBarycenter() +
> > "\nvolume= " + box.getSize() +
> > "\nsurface area= " + box.getBoundarySize());
> >
> > // Move the box center to the pyramid center and lengthen it
> > // along the z-axis
> > AffineTransformMatrix3D mat = AffineTransformMatrix3D.identity()
> > .translate(region.getBarycenter())
> > .scale(0.75, 0.75, 4);
> >
> > box.transform(mat);
> >
> > System.out.println("\n# Transformed Box" +
> > "\nbarycenter= " + box.getBarycenter() +
> > "\nvolume= " + box.getSize() +
> > "\nsurface area= " + box.getBoundarySize());
> >
> > // Subtract the box from the pyramid. The pyramid is modified but
> > // the box is not.
> > region.difference(box);
> >
> > System.out.println("\n# Pyramid with hole" +
> > "\nbarycenter= " + region.getBarycenter() +
> > "\nvolume= " + region.getSize() +
> > "\nsurface area= " + region.getBoundarySize());
> >
> > // Print out the vertices of the faces that make up the modified
> > // pyramid. The faces are not simplified; they represent the internal
> > // structure of the bsp tree.
> > System.out.println("\n# Faces");
> > for (ConvexSubPlane face : region.boundaries()) {
> > System.out.println(face.getVertices());
> > }
> >
> > ________________________________
> > From: Gilles Sadowski <gillese...@gmail.com>
> > Sent: Friday, August 16, 2019 11:37 AM
> > To: Commons Developers List <dev@commons.apache.org>
> > Subject: Re: [geometry] GEOMTRY-32 Feedback Requested
> >
> > Hello Matt.
> >
> > Thanks for continuing to work on this.
> >
> > Le jeu. 15 août 2019 à 17:56, Matt Juntunen
> > <matt.juntu...@hotmail.com> a écrit :
> > >
> > > Hi all,
> > >
> > > I've been working on the BSP tree refactor and general API cleanup for a
> > > while now and I finally have the core and Euclidean classes complete. I
> > > have the code in a draft PR on github [1] and I'm hoping to get as much
> > > feedback on it as I can.
> >
> > There is a spurious change of ".travis.yml". Also there are wrong EOL
> > characters
> > in that file:
> > ---CUT---
> > $ file .travis.yml
> > .travis.yml: ASCII text, with CRLF line terminators
> > ---CUT---
> > Perhaps you should merge or rebase on "master".
> >
> > > Everything is in its final state from my point of view with the exception
> > > of the spherical module, which still needs to be switched over to the new
> > > API. Here are some highlights of the new code:
> > >
> > > * More user-friendly API
> > > * Does not require users to make unchecked casts to implementation
> > > types.
> > > * BSP tree classes are much more powerful and easy to use. State
> > > is managed internally so that users do not need to be experts in BSP
> > > trees to avoid corrupting the data structures.
> > > * Uses builder pattern instead of large factory methods to build
> > > complicated geometries.
> >
> > That would suit me. ;-)
> > Actually, I'd like to try it with a simple geometry (i.e. build a sphere).
> > Where should I start looking?
> >
> > > * Most classes are immutable.
> > > * A general-purpose AttributeBSPTree class is available so that users
> > > can associate arbitrary data with spatial partitionings. I'm picturing
> > > this being used for spatial data lookups, painter's algorithms, etc.
> >
> > Can it be associated with a surface element?
> >
> > > * All geometric types now support arbitrary transforms (eg, rotate,
> > > scale, translate, etc) via the Transform interface.
> > > * The Transform interface is greatly simplified (GEOMETRY-24). It is
> > > now functional and simply extends java.util.Function.
> >
> > Nice.
> >
> > > * Better performance. My highly unsophisticated stdout benchmarking
> > > put the new code at about 3-4 times faster than the old when performing
> > > boolean operations on 3D regions.
> >
> > Impressive.
> >
> > It would be great to create a maven module for systematizing the benchmarks;
> > see e.g. what is being done in "Commons RNG":
> >
> > https://gitbox.apache.org/repos/asf?p=commons-rng.git;a=tree;f=commons-rng-examples/examples-jmh
> >
> > A "commons-geometry-examples" module would be a place to collect
> > useful code, e.g. simple "howtos" (like the one I mentioned above) and
> > conversion routines from/to popular formats, without the requirement of
> > backwards compatibility (even between minor releases).
> >
> > Best,
> > Gilles
> >
> > >
> > > Regards,
> > > Matt
> > >
> > > [1] https://github.com/apache/commons-geometry/pull/34
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
