Skip to content
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

Update padFootprint function #108

Open
wants to merge 5 commits into
base: devel
Choose a base branch
from
Open

Conversation

Nisarg236
Copy link
Member

@Nisarg236 Nisarg236 commented Dec 25, 2024

Previously this function was scaling the footprint with respect to the coordinate frame 0,0 which could result in distortion. Also when the centroid is having large offset with 0,0 then it will just displace the footprint instead of padding.

Now it scales with respect to the centroid of the footprint.

image
image
another case (10,10), (10,12), (12,12) , (12,10)
image
image

another case (0,0), (0,2), (2,2), (2,0)
image
image

I have also tested the behaviour with negative values of padding (shrinking the footprint)

@Nisarg236 Nisarg236 marked this pull request as ready for review December 25, 2024 10:15
corot

This comment was marked as duplicate.

Copy link
Collaborator

@corot corot left a comment

Choose a reason for hiding this comment

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

3 things

The 1st example looks suspicious: doesn't look like padded 10 cm but 2 or 3 🤔

This looks similar to this commit
Do both provide the same result? If so, this is simpler

Keep the formatting of the files you modify. i.e. never mix stiles

@Nisarg236
Copy link
Member Author

3 things

The 1st example looks suspicious: doesn't look like padded 10 cm but 2 or 3 🤔

This looks similar to this commit Do both provide the same result? If so, this is simpler

Keep the formatting of the files you modify. i.e. never mix stiles

Hi you are right it wasnt 0.1 meters because it was scaling by percentage, I changed it to scale by meters and updated the image.
and now the functionality is the same as the one you shared.

@corot corot closed this Dec 26, 2024
@corot corot reopened this Dec 26, 2024
@corot
Copy link
Collaborator

corot commented Dec 26, 2024

Hi you are right it wasnt 0.1 meters because it was scaling by percentage, I changed it to scale by meters and updated the image. and now the functionality is the same as the one you shared.

So the outcome is now the same as with this commit?
Then better to use the commit,,, as the logic is much simpler 🤔

@renan028
Copy link
Member

Hi you are right it wasnt 0.1 meters because it was scaling by percentage, I changed it to scale by meters and updated the image. and now the functionality is the same as the one you shared.

So the outcome is now the same as with this commit? Then better to use the commit,,, as the logic is much simpler 🤔

@corot I think you are missing the part he calculates the centroid of the polygon.

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.

3 participants