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

Remarks #1

Open
Fil opened this issue Jul 6, 2020 · 3 comments
Open

Remarks #1

Fil opened this issue Jul 6, 2020 · 3 comments

Comments

@Fil
Copy link

Fil commented Jul 6, 2020

I like the way this is going forward! The demos work well, and the computation at each step is quite simple (which means fast and understandable).

Can we explore changes in the API:

  • set the values in one go (ie .extent([extent]) rather than x0 x1…)

  • (optionally?) specify the cushion's size as a proportion of the dimensions (e.g. %) rather than pixels.

  • have default values that already do some work (e.g. extent [0,0,960,500], cushion "2%")

In terms of code:

  • no need to specify numDimensions in the initialization: just set the list of coordinates to inspect whenever the x,y,z accessor is set (that is, when by default it's infinite)

  • check undefined/null/NaN instead of hasownproperty?

Also, I'd love to use a (convex) polygon rather than just an horizontal rectangle, but I imagine it might make things a bit slower and it's not that easy to specify in 3D.

@vasturiano
Copy link
Owner

I like the way this is going forward! The demos work well, and the computation at each step is quite simple (which means fast and understandable).

Nice!

Can we explore changes in the API:

  • set the values in one go (ie .extent([extent]) rather than x0 x1…)

The reason why it's the way it is is so that you can specify each restriction independently, and even leave out some of the sides unrestricted, if desired. Think of having a container with an open top for example. Handling it in a single shot would be doable, but probably cumbersome to have to repeat the default Infinity settings, etc. Also, in some cases you may want to update the container in just one direction, like extending the right side on window resize for instance. It's nice to be able to do so without having to remember the height settings and so forth. Not a deal breaker, but I think having them independent may actually be a bit cleaner.

  • (optionally?) specify the cushion's size as a proportion of the dimensions (e.g. %) rather than pixels.

That's a cool idea. But, what do we do when some of the sides are unrestricted? Can't take a relative portion of Infinity. Another thing that comes to mind is that you will most often have different width and height values. This would lead to an asymmetrical horizontal vs vertical cushion effect, which I suspect it's not really what we want.

  • have default values that already do some work (e.g. extent [0,0,960,500], cushion "2%")

It's hard to guess default container sizes. 😄 Currently it defaults to all unrestricted (Infinity). I think that's probably the only safe bet to not have users scratch their heads as to why nodes are not making it past a certain area.
As for the cushion settings, the default strength is set to 0.01 (px acceleration). It does a little but not much, just enough to gently displace the escaping nodes away from the wall. And this also depends on what kind of energy is going on in the system at the time.
The cushion thickness defaults to 0 which essentially switches it off. If we don't have relative cushion widths it's again hard to guess because an absolute val could be a lot or a little.

Anyway, I'm happy to change these to smarter default values if we want.

In terms of code:

  • no need to specify numDimensions in the initialization: just set the list of coordinates to inspect whenever the x,y,z accessor is set (that is, when by default it's infinite)

Since those properties are node accessor functions, it's difficult to verify whether they've been set or not. Even invoking those functions would not give a 100% certainty as they may return Infinity for some nodes but not others. It would also be a bit more of an expensive verification than by caching the numDimensions. Moreover, there is also the issue when the consumer (erroneously or because it may be switching back and forth between 2D and other simulation modes) sets the accessors for dimensions that don't belong in the current mode (like z0 for 2d). In that case the lib would be doing unnecessary work of applying restrictions to an unused dimension.

All in all it may be more efficient for the force to have the context of how many dimensions are in use, and make choices according to it.

  • check undefined/null/NaN instead of hasownproperty?

TIL just how bad hasOwnProperty is for perf. https://jsperf.com/hasownproperty-vs-in-vs-undefined
Good call, I'm def changing that. 👍

You could almost catch all with isNaN, but alas #jswtf:

isNaN(undefined)
true
isNaN(null)
false

Also, I'd love to use a (convex) polygon rather than just an horizontal rectangle, but I imagine it might make things a bit slower and it's not that easy to specify in 3D.

Oh yes, that would be great! Imagine being able to specify the container perimeter as an svg path definition. Or a Geometry in 3D.
I think it does complicate the math dramatically, and undoubtedly bring a performance penalty. Maybe this is a different force plugin, for a future "rainy day". ☔

@Fil
Copy link
Author

Fil commented Jul 7, 2020

does complicate the math dramatically

Actually not that much: for each side (face) of the polygon we would precompute a normal vector n and plane value v, and at each run we'd compare a dot product to the value : if (dot(n, d) < v) { go in the direction of n }; the performance penalty would be just 1 multiplication per dimension. And it doesn't need to be a "polygon", just a set of hyperplanes.

@Fil
Copy link
Author

Fil commented Jul 8, 2020

I've generalized force-limit as forceWalls, which takes a list of walls defined by a reference point, a normal, and (optionally) a cushion. The way it's coded, it should work as well in any dimensions: 1D, 2D, 3D and more(!).

https://observablehq.com/d/21d2053b3bc85bce
https://observablehq.com/d/66b419d51abee995

The API is a bit rough, but we could add helpers to create the walls from an extent, a polygon, a list of vertices (with their polygonHull)…

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

No branches or pull requests

2 participants