Allow hull() to process listOfPoints3 in addition to path2, geom2 and geom3

[Hermann-SW]

All Submissions:

• [x] Have you followed the guidelines in our Contributing document?
• [x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• [x] Does your submission pass tests?

Note: please do NOT include build files (those generate by the build-xxx commands) with your PR,

Thank you for your help in advance, much appreciated !

[Hermann-SW]

New testcase is for "n=5" and "≤ n" in this JSCAD app: https://stamm-wilbrandt.de/lattice_sphere_cmp.html In that app "hullFromPoints3(listofpoints)" does the job of this pull request.

[z3dev]

@Hermann-SW Thanks again.

There are many ways to make a cake. I'm thinking that we could offer users more recipes if we reorganize the hull functions slightly. Something like this...

hull() // takes lists of path2, geom2, geom3
  calls hullGeom3() // with list of geom3
     calls hullPoints3() // with unique set of 3D vertices

Today, only hull() is available in the API, but I'm thinking that all of these can exposed in the API.

And we do the same for 2D hulls as well.

@platypii Is this what you were thinking?

[platypii]

My thought was to leave hull as is. It's okay if hull accepts a list of points to me. One thing I don't like about this api change though, is that nowhere else do we treat "list of points" as a geometry anywhere else, so it makes hull inconsistent with the rest of the API.

Personally, my preferred design for this API would actually be something like geom3.createConvex(points) or something similar to that. It actually is the same operation as hull on points, but I think makes it much more intuitive for users who think "I want to define a 3D shape based on its outer points" whereas now if you wanted to make an arbitrary 3D shape from points, you need to think in "polygons".

That being said, this change looks fine, and I appreciate the test @Hermann-SW! I am happy to have this change in whatever form people prefer.

[Hermann-SW]

My thought was to leave hull as is. It's okay if hull accepts a list of points to me. One thing I don't like about this api change though, is that nowhere else do we treat "list of points" as a geometry anywhere else, so it makes hull inconsistent with the rest of the API.

OK.

Personally, my preferred design for this API would actually be something like geom3.createConvex(points) or something similar to that. It actually is the same operation as hull on points, but I think makes it much more intuitive for users who think "I want to define a 3D shape based on its outer points" ...

I need help then, as I tried to implement that function and dev server stops working. It was easy to just modify hull.js, here new files need to be created and correctly integrated ... Without the index* file changes, geom3.createConvex() is undefined. With the changes working hull demo below stops working with:

TypeError: geom3.isA is not a function

toUniquePoints/<@blob:http://192.168.10.34:8081/b266d022-a331-429c-9d18-52fc42328a3d:37495:22

toUniquePoints@blob:http://192.168.10.34:8081/b266d022-a331-429c-9d18-52fc42328a3d:37492:14

hullGeom3@blob:http://192.168.10.34:8081/b266d022-a331-429c-9d18-52fc42328a3d:35950:32

hull@blob:http://192.168.10.34:8081/b266d022-a331-429c-9d18-52fc42328a3d:35833:13

main@blob:http://192.168.10.34:8081/b266d022-a331-429c-9d18-52fc42328a3d line 2253 > Function:16:11

instanciateDesign@blob:http://192.168.10.34:8081/b266d022-a331-429c-9d18-52fc42328a3d:1780:49

rebuildSolids@blob:http://192.168.10.34:8081/b266d022-a331-429c-9d18-52fc42328a3d:1855:41

rebuildGeometryWorker/self.onmessage@blob:http://192.168.10.34:8081/b266d022-a331-429c-9d18-52fc42328a3d:1897:24

hermann@j4105:~/OpenJSCAD.org/packages/modeling/src/geometries/geom3$ git status On branch master Your branch is up to date with 'origin/master'.

Changes not staged for commit: (use "git add ..." to update what will be committed) (use "git restore ..." to discard changes in working directory) modified: index.d.ts modified: index.js

Untracked files: (use "git add ..." to include in what will be committed) createConvex.d.js createConvex.js

no changes added to commit (use "git add" and/or "git commit -a") hermann@j4105:~/OpenJSCAD.org/packages/modeling/src/geometries/geom3$

hermann@j4105:~/OpenJSCAD.org/packages/modeling/src/geometries/geom3$ git diff diff --git a/packages/modeling/src/geometries/geom3/index.d.ts b/packages/modeling/src/geometries/geom3/index.d.ts index 436f6e01..da51a7b4 100644 --- a/packages/modeling/src/geometries/geom3/index.d.ts +++ b/packages/modeling/src/geometries/geom3/index.d.ts @@ -1,6 +1,7 @@ export { default as clone } from './clone' export { default as create } from './create' export { default as fromPoints } from './fromPoints' +export { default as createConvex } from './createConvex' export { default as fromCompactBinary } from './fromCompactBinary' export { default as invert } from './invert' export { default as isA } from './isA' diff --git a/packages/modeling/src/geometries/geom3/index.js b/packages/modeling/src/geometries/geom3/index.js index 2591de08..3ca83da5 100644 --- a/packages/modeling/src/geometries/geom3/index.js +++ b/packages/modeling/src/geometries/geom3/index.js @@ -23,6 +23,7 @@ module.exports = { clone: require('./clone'), create: require('./create'),

• createConvex: require('./createConvex'), fromPoints: require('./fromPoints'), fromCompactBinary: require('./fromCompactBinary'), invert: require('./invert'), hermann@j4105:~/OpenJSCAD.org/packages/modeling/src/geometries/geom3$

hermann@j4105:~/OpenJSCAD.org/packages/modeling/src/geometries/geom3$ cat createConvex.d.js 
import Geom3 from './type'
import Vec3 from '../../maths/vec3/type'

export default createsConvex

declare function createConvex(points: Array<Array<Vec3>>): Geom3
hermann@j4105:~/OpenJSCAD.org/packages/modeling/src/geometries/geom3$ 
hermann@j4105:~/OpenJSCAD.org/packages/modeling/src/geometries/geom3$ cat createConvex.js 
const create = require('./create')

const fromPoints = require('./fromPoints')

const hullGeom3 = require('../../operations/hulls/hullGeom3')

/**
 * Construct a new convex 3D geometry from a list of points.
 * @param {Array} listofpoints - list of points to construct convex 3D geometry
 * @returns {geom3} a new geometry
 * @alias module:modeling/geometries/geom3.createConvex
 */
const createConvex = (listofpoints) => {
  if (Array.isArray(listofpoints) && listofpoints.every((p) =>
       (Array.isArray(p) && p.length===3 && p.every((c) =>
         (typeof(c)==="number"))))) {
           return hullGeom3([[ create(),
             fromPoints(listofpoints.map((p) => [p,p,p])) ]])
  }
}

module.exports = createConvex
hermann@j4105:~/OpenJSCAD.org/packages/modeling/src/geometries/geom3$ 

Test script for "npm run dev":

const jscad = require('@jscad/modeling')
const { sphere } = jscad.primitives
const { hull } = jscad.hulls
const { geom3 } = jscad.geometries

function main() {
  let out = []
  for(x=-2;x<=2;++x)
    for(y=-2;y<=2;++y)
     for(z=-2;z<=2;++z)
        if (x*x+y*y+z*z <= 5)
          out.push([x,y,z])
   return hull(out)       
//   return geom3.createConvex(out)       
}

module.exports = { main }

[platypii]

I would do something like this. Also @z3dev what do you think about the name geom3.fromPointsConvex?

const quickhull = require('../../operations/hulls/quickhull')
const create = require('./create')
const poly3 = require('../poly3')

/**
 * Construct a new convex 3D geometry from a list of points.
 * @param {Array} listofpoints - list of points to construct convex 3D geometry
 * @returns {geom3} a new geometry
 * @alias module:modeling/geometries/geom3.createConvex
 */
const fromPointsConvex = (listofpoints) => {
  if (!Array.isArray(listofpoints)) {
    throw new Error('the given points must be an array')
  }

  const faces = quickhull(listofpoints, { skipTriangulation: true })

  const polygons = faces.map((face) => {
    const vertices = face.map((index) => listofpoints[index])
    return poly3.create(vertices)
  })

  return create(polygons)
}

module.exports = createConvex

[Hermann-SW]

I created new #1344 based on @platypii last comment.