Skip to content

Fix centroid calculation to return bounding box center for multiple vectors #3545

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.

Sign up for GitHub

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

Closed
daniil-loban wants to merge 2 commits into from
Closed

Fix centroid calculation to return bounding box center for multiple vectors #3545

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a12770f
Fix centroid calculation to return bounding box center for multiple v…
Dec 30, 2025
b61af57
Merge branch 'master' into fix_centroid_calculation
daniil-loban Jan 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions node-graph/nodes/vector/src/vector_nodes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2336,6 +2336,14 @@ async fn centroid(ctx: impl Ctx + CloneVarArgs + ExtractAll, content: impl Node<
return DVec2::ZERO;
}

// Calculate the bounding box of all vectors in the table
let bounding_box = vector.bounding_box(DAffine2::IDENTITY, false);
if let RenderBoundingBox::Rectangle([min, max]) = bounding_box {
// Return the center of the bounding box
return (min + max) / 2.;
}

// Fallback to the original method if bounding box calculation fails
Copy link
Copy Markdown
Contributor

@0HyperCube 0HyperCube Dec 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@daniil-loban daniil-loban Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I need to delete the following code in this case?

Copy link
Copy Markdown
Contributor

@0HyperCube 0HyperCube Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

result with star in centroid node graph

Copy link
Copy Markdown
Contributor Author

@daniil-loban daniil-loban Jan 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

image

Copy link
Copy Markdown
Member

@Keavon Keavon Jan 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bottom left shape has the largest area so the center of mass will be concentrated towards it, as we observe.

Copy link
Copy Markdown
Member

@Keavon Keavon Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@daniil-loban daniil-loban Jan 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's exactly how it's calculated (Discord: https://discord.com/channels/731730685944922173/1134981447539761273/1456461715872026809 )

image image

Copy link
Copy Markdown
Contributor Author

@daniil-loban daniil-loban Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

@Keavon Keavon Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is planned, yes. Feel free to also open an issue to track that so we don't forget. Thanks!

Copy link
Copy Markdown
Contributor

@0HyperCube 0HyperCube Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daniil-loban If you have the bounding box -> centroid then it will find the centre of the bounding box.

image image

// All subpath centroid positions added together as if they were vectors from the origin.
let mut centroid = DVec2::ZERO;
// Cumulative area or length of all subpaths
Expand Down
Loading