Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/fit into bounds 3 #206

Closed
wants to merge 5 commits into from
Closed

Conversation

nkint
Copy link
Contributor

@nkint nkint commented Feb 19, 2020

feat into bounds 3, closes #202

Should there be an error or a warning if the source or destination bounding box has some 0 dimensions?

@postspectacular
Copy link
Member

Hi @nkint - firstly, thank you very much for pushing this forward, but there're a few things which need to be addressed/changed before this can be merged:

  • The 2D version fitIntoBounds2 operates on shape types, not directly on point arrays and
  • The existing 2D version also scales the given shape proportionally, using a uniform scale factor to "fit" the source shape into the smallest dimension of the target bounds. Your new 3D version scales non-proportionally, which is fine in general, but not the same behavior as the 2D one and I'd really like to keep that
  • I'm somewhat confused by all those checks in the transformV344 function. Could this not be skipped and handled like the existing transformPoints() functions (maybe also moved to that same source file)?

So my main questions right now are:

  • Where to put that new function in its current form? I.e. the main thi.ng/geom package is not supposed to be publicly dealing with raw point arrays, only with shape types (which might consist of point collections). On the other hand, there're hardly any 3D shape types implemented so far (only a half-baked AABB, Sphere thus far), so maybe there could be new (more low-level) geom-fit package for raw arrays?
  • How to deal / fix discrepancies re: uniform vs. non-uniform scaling in 2D/3D versions?

The 3D tscale factor calculation is actually better/safer than the 2D one, but the 2D version then picks the smallest scale factor. However, the problem here is that safeDiv could return 0 for some scale-vector components and thus simply choosing the smallest value would be wrong. We need to use the smallest, non-zero value of the tscale vector components as scale factor.

Hope these comments make sense! Please let me know your thoughts...

@postspectacular
Copy link
Member

@nkint - I've added & refactored both the 3D and 2D versions of fitIntoBounds and also added the Point3 3D pointcloud container incl. the necessary operations required. So if you want to use fitIntoBounds3 for just a point array, do something like the following:

import * as g from "@thi.ng/geom";

const pts = [[-1,1,1], [1,1,1]];

g.fitIntoBounds3(g.points3(pts), g.aabbFromMinMax([-8,-8,-8], [8,8,8])).points
// [ [ -8, 0, 0 ], [ 8, 0, 0 ] ]

@nkint nkint deleted the feat/fit-into-bounds-3 branch February 26, 2020 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[geom] fitIntoBounds3
2 participants