Fix centroid calculation to return bounding box center for multiple vectors#3545
Fix centroid calculation to return bounding box center for multiple vectors#3545daniil-loban wants to merge 2 commits intoGraphiteEditor:masterfrom
Conversation
| return (min + max) / 2.; | ||
| } | ||
|
|
||
| // Fallback to the original method if bounding box calculation fails |
There was a problem hiding this comment.
The bounding box only fails if there are no segments to take the bounding box. In this case, the code below isn't going to work either.
Essentially this changes the implementation to just always return the centre of the bounding box.
There was a problem hiding this comment.
Do I need to delete the following code in this case?
There was a problem hiding this comment.
As kindly pointed out by elbertronnie on #3544, there was a typo in the pathseg_to_parametric_polynomial function that caused incorrect results. With the fix in #3562 it is now resolved:
There was a problem hiding this comment.
I may be wrong about the concept, but in my opinion the centroid is not where it should be.
I think that the general centroid should act according to the same principles as the single centroid — find the center.
There was a problem hiding this comment.
The bottom left shape has the largest area so the center of mass will be concentrated towards it, as we observe.
There was a problem hiding this comment.
Centroid computes the center of mass. It sounds like you are expecting the center of the bounding box. You can add a Bounding Box node before Centroid to get the center of mass of the rectangular bounding box, which would have your desired effect.
There was a problem hiding this comment.
That's exactly how it's calculated (Discord: https://discord.com/channels/731730685944922173/1134981447539761273/1456461715872026809 )
There was a problem hiding this comment.
Centroid computes the center of mass. It sounds like you are expecting the center of the bounding box. You can add a Bounding Box node before Centroid to get the center of mass of the rectangular bounding box, which would have your desired effect.
Perhaps then it's worth simply creating an additional node that will calculate the geometric center?
There was a problem hiding this comment.
That is planned, yes. Feel free to also open an issue to track that so we don't forget. Thanks!
There was a problem hiding this comment.
@daniil-loban If you have the bounding box -> centroid then it will find the centre of the bounding box.
Keavon
changed the title
Ezbaze |
3 participants
I noticed that the centroid for the Bounding Box node is calculated incorrectly when there are multiple shapes passed via Flatten Path. However, the rectangle itself is calculated correctly. It seemed logical to me to use the bounding_box call to get the rectangle, since I believe its center will be the correct centroid.
Result:
Further calculations may not be necessary, but I left them in just in case.
Fixes #3544