Skip to content

added Offset method to selection class#740

Merged
cameronwhite merged 4 commits into
PintaProject:masterfrom
zWolfrost:master
Mar 7, 2024
Merged

added Offset method to selection class#740
cameronwhite merged 4 commits into
PintaProject:masterfrom
zWolfrost:master

Conversation

@zWolfrost

Copy link
Copy Markdown
Contributor

This PR aims to close my issue (#661).
It doesn't implement any GUI options, but hopefully it makes the job much easier for people that know how to add the buttons.
I have tried my best to respect the project coding practices but i am kinda new to doing pull requests, so i would really like if this was tested first.
To get into technical details, i have chosen the "jtMiter" JoinType as i think it's the most fitting between these options.

@cameronwhite cameronwhite left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for working on this!
I'll need to do some testing of it (and also think about how to expose it in the UI), but the code looks good to me. Just had one minor style suggestion noted in a comment

Comment thread Pinta.Core/Classes/DocumentSelection.cs Outdated
@cameronwhite

Copy link
Copy Markdown
Member

The changes look good, thanks!
I tried testing this out by just hacking it into the selection tool to always expand the selection, and it behaved as expected - I just noticed that the surface parameter is unused and could be removed

@zWolfrost

zWolfrost commented Mar 6, 2024

Copy link
Copy Markdown
Contributor Author

Hi, just for carification, i added the unused surface parameter because the "Invert" method of the selection object also has it unused, so i figured i should add it as well. Anyway, i'm glad it works. Do you want me to remove the parameter?

@cameronwhite

Copy link
Copy Markdown
Member

Thanks, yes please remove that parameter (and also may as well do this for the Invert() method too)

@zWolfrost

zWolfrost commented Mar 6, 2024

Copy link
Copy Markdown
Contributor Author

There you go. I didnt remove it from invert method as well because that would require to edit every call of it and remove the paramether there as well, and my pc is not available at the moment.

@cameronwhite cameronwhite merged commit 5231624 into PintaProject:master Mar 7, 2024
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