Skip to content

Fixes method for appending bezpath to the 'VectorData' and the 'Scatter Points' node implementation. #2645

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

indierusty
Copy link
Collaborator

Fixes mistakes I made in 'append_bezpath()' method #2608 and 'Scatter Points' #2634 node refactoring.

@indierusty indierusty changed the title Fix mistake in the 'append_bezpath()' method and the 'Scatter Points' node implementation. Fixes method for appending bezpath to the 'VectorData' and the 'Scatter Points' node implementation. May 14, 2025
@indierusty
Copy link
Collaborator Author

Screenshot 2025-05-15 at 11 10 46 AM Screenshot 2025-05-15 at 11 10 40 AM

@indierusty
Copy link
Collaborator Author

Though refactoring 'Scatter Points' node changed look of the 'Red Dress' demo artwork. @Keavon

@Keavon
Copy link
Member

Keavon commented May 15, 2025

That's unfortunate. It seems to have affected all the Red Dress scatter points layers, not just the one (which I assumed was due to numerical instability). This leads me to believe something may be being altered more fundamentally which you could avoid by debugging the PR's changes and finding what part of the logic has resulted in a different result.

@indierusty
Copy link
Collaborator Author

indierusty commented May 15, 2025

That's unfortunate. It seems to have affected all the Red Dress scatter points layers, not just the one (which I assumed was due to numerical instability). This leads me to believe something may be being altered more fundamentally which you could avoid by debugging the PR's changes and finding what part of the logic has resulted in a different result.

I tried debugging but could not understand why it really changes that much. I will try to find the cause again but It would be helpful if you give a quick look to the changes and maybe point out the lines which could be causing this.

NOTE: I've not made any chages to the possion_disk_sample() algorithm it self but the only to checking the insideness of a point to the bezpaths.

@Keavon
Copy link
Member

Keavon commented May 15, 2025

Just from my quick glance, some of the changes in bezpath_algorithms.rs look like of sus for affecting the output.

@indierusty
Copy link
Collaborator Author

indierusty commented May 15, 2025

I tried to do the point insidness checking as it was done before using kurbo and only fixing the bug in the 'Interactive Snake Pebbles', which was due to incorrect insidness checking with bounding boxes, but yet the behaviour was same to the current behaviour.

I will give it another try and see why this happens. Thanks.

@indierusty indierusty marked this pull request as draft May 15, 2025 06:06
@indierusty indierusty marked this pull request as ready for review May 17, 2025 09:17
@indierusty indierusty marked this pull request as draft May 18, 2025 02:39
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

Successfully merging this pull request may close these issues.

2 participants