-
Notifications
You must be signed in to change notification settings - Fork 79
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
Implement uniform random point in polygon, triangle and circle #83
base: feature/no-org
Are you sure you want to change the base?
Conversation
Second column shows uniform distribution working on circles, triangles, and polygons. Rotated rectangle *becomes* a polygon. Random point and sample-uniform examples are present just to confirm they are also happy.
This is awesome, @dgtized - probably one of the best PRs I've ever got on this project so far! Well done also for referencing the same article by Martin Roberts (re: Will take a more detailed look over the next couple of days & report back! 🙏🙏 |
Any thoughts on this? As it stands, it adds functionality but does not adjust existing code, but happy to make further changes if that would assist in merging, or help out in any other way. Very much appreciate how issues can lost in among various projects, so definitely understand if it's just waiting on sufficient time to focus on it. |
This PR makes the assumption that g/area is positive for triangles to equal weight the random selection. However it's not clear that the triangles created from t/tessellate will always be in clockwise orientation, so it probably needs to use the absolute value of the area. This is related to #88 |
Tessellates into triangles, randomly selects a triangle weighted by area, | ||
and then samples a point inside of that triangle uniformly." | ||
(->> (g/tessellate _) | ||
(map t/triangle2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If #89 is merged, this conversion to triangles will no longer be necessary.
I've created a couple of sketches using the
g/random-point-inside
sampling method fromISample
, and found it to be very useful, so thank you for providing that functionality.Unfortunately I discovered some limitations. Firstly, the method was not implemented on polygons, or rectangles that had been rotated (which upscales to a polygon). In the process of addressing that, I discovered that the implementation for triangles is biased towards the centroid and not distributed uniformly throughout. Finally, I determined that the method for circle is also biased towards the centroid and not uniform.
The screenshot below is generated using the additional example script
examples/svg/uniform_distribution.clj
and helps demonstrate the bias visually. The clumping bias is quite visible on both the circle and triangle examples.The first column shows distributions generated using the existing
g/random-point-inside
routines. The second column shows all the functionality using the uniform methods. Lastly the final two columns demonstrate both thesample-uniform
andg/random-point
on hull methods appear to be functioning uniformly.The algorithm for polygons involves tessellating into triangles, and then sampling the triangles weighted by area. I suspect the math routines added for weighted probability distributions belong in
thi.ng.math.core
, but wanted to include them here in the interest of a single PR to merge. If this is accepted, I am happy to create the PR necessary to migrate it into the other project.I think my preference would be to replace the
random-point-inside
implementations to use the uniform algorithms, however, that may break existing works generated using these methods. I have tried to think of an alternate name likerandom-point-inside-uniform
to add to the interface, but have not been particularly happy with any names I have thought of. Adding a new interface seems like it would allow a common access method without dropping backwards compatibility but requires a good name. In the interest of sharing the results back with the community I elected to just add each as a helper method, but re-use therandom-point-inside
interface for polygons as no existing functionality can depend on it biasing in a particular way.One last note,
random-point-inside-triangle2
has only been tested on 2d triangles but should work fine in 3d, as the bias weights should work on vec3's just as well as vec2's.Hopefully this functionality is useful for others! Thanks again for this great project.