Points are unevenly distributed within a triangle. I haven’t tried it, but I am pretty sure that this can be easily fixed by changing this line to “lamda = Math.sqrt(Scalar.RandomRange(0, 1));”.
(See also the Wikipedia page on triangular distributions.)
If the mesh contains many small triangles, points are unevenly distributed across triangles.
To see this, change the the number of segments in the playground example from 1 to 32.
Then we get no particles around the south pole but a high density around the north pole.
This is due to some hacky/broken logic in method _calculateDensity(...):
First the density for the small triangles around the poles is floored to 0 (line 603) and near the end of the method (line 615) the extraPoints are distributed over the first few triangles, which happen to be around the north pole.
A cleaner solution would work like this:
Whenever the division in line 603 produces a non-integer quotient, we assign the floor or the ceiling of that quotient, with a probability given by the fractional part.
Example: Assume the quotient is 4.56. Then we assign 4 points for sure and an additional point with a probability of 56%.
A straight-forward implementation of this approach (where the probabilistic decisions for different triangles are independent) may lead to a total number of points that is a little to high or a little too low. But the approach can be refined to avoid this.
(As an alternative, we could give up the requirement that the given number of points must be obeyed precisely. The given number would just be the expected mean number of points. We could use a Poisson distribution with that number as its parameter. The advantage would be that a Poisson distribution can be broken down from the mesh to the triangles very easily. In our example above we would simply draw a Poisson-distributed random variable with parameter 4.56. )
If you are interested, I would volunteer to implement fixes for issues 1 and 2.
And while we are at this class already, I have some more remarks (which are, however, not bugs):
The computation of triangle areas in lines 590 to 597 can be simplified. The area is just half the length of the cross product of vec0 and vec1.
When I move the camera closer to my mesh I would have expected the particles to grow. Similarly when moving the camera away, the particles should IMHO be rendered smaller. But actually the particles keep their “rendered size”. This makes the “intuitive density” unnaturally dependent on the camera distance. Did I miss a flag for changing this behavior? Or is the current behavior actually intended?
The code for filling a volume also feels quite hacky. But admittedly I do not have a truly better solution.
One approach would be to just randomly fill the mesh’s bounding box with points and to drop those points that are outside the mesh (recognizable by an odd number of intersections between the mesh and the line from the point to some fixed “central point”). This would probably work quite well for compact closed shapes but would be inefficient for other shapes. (Actually intersection detection is what GPUs are good at.)
So as I wrote above, I could fix issues 1 and 2 if there is interest in having this fixed. And I would also like to know what you think about issues 3, 4, and 5.
OK, I started with issue 1. Got a fork of Babylon on Github and a clone on my machine. Got the development environment to run on my machine, made my 1-line change, opened http://localhost:1337/#UI95UC#2446, found that it looks better than in the playground.
(BTW, Babylon.js docs says that playground examples can be tested at “localhost:1338#…”. That did not work for me. I had to use port 1337 for this. Is this a mistake in the docs?)
Now I am having two problems with this pull request. Could you please give me some advice?
(a) bjsplat (a robot?) asked me to label my PR with “bug” (or some other values that do not apply). Github documentation tells me to click “Labels” in the right sidebar and then to select a label. But unfortunately nothing happens when I click “Labels” in the right sidebar of the PR. This is apparently not an active element. Do you have any idea?
(b) Two of the automatic tests were not successful. But the details only say “Bash exited with code ‘1’”. Not so helpful. Can I ignore this?
(One more note: I temporarily switched the PR to “draft”, hoping that this might solve problem (a). It didn’t. But could that be the reason for the test failure (b)?)
Anyway, could you have a look at the PR? Then I’ll work on issue 2 tomorrow.
The test passed, so i don’t see an issue there. Most have been a temporary asset loading issue, because i don’t remember this test in the flaky list. Is it new?
…aaand another PR for issue 3. This one is not a bug fix, just a code simplification. I don’t know your policy regarding mere cleanup changes.
This PR might conflict the one for issue 2. So I would re-base it once the PR for issue 2 is merged.
And a side note: I am quite impressed by your development and project-management environment. In particular the automatically created playground clones for pull requests are really cool!
