-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Implement Magic Wand Tool (#128) #854
Conversation
} | ||
|
||
brushItem()->setTileRegion(mSelectedRegion); | ||
document->setSelectedArea(mSelectedRegion); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line seems wrong. The selected area is already applied to the document by the ChangeSelectedArea
undo command created and pushed on the undo stack above.
Also, this seems like the wrong moment to set the tile region of the brush item. Shouldn't that be done in the tilePositionChanged
function to provide a preview of the area that's going to be selected?
To fix the AppVeyor continuous-integration build you should add |
OK, Done. |
Add, | ||
Subtract, | ||
Intersect | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have these selection modes here but you're not using them. If the tool is working so far, why not try and implement that functionality like in the TileSelectionTool
?
OK |
I'm not fully sure how the selection modes work in the actual program (as in how you change it) |
Please see if you can just derive this by reading the code in |
Oh I see, through modifiers on the keyboard. |
…tion Tool. However, the shift modifier does not work yet due to problems with TilePainter::computeFillRegion
I've added the keyboard modifiers. However the shift (add) modifiers doesn't work because computefillRegion only computes it within the selected region. |
Alright, that will require some changes to |
I forgot to say, the magic wand tool will also need a new icon for it. Currently I've just got the icon set to the same one as bucket fill. |
Fortunately, the GIMP also has a magic wand tool so we can just use the same icon. Most of the other icons are also taken from the GIMP. |
Hmm, I don't know where to get the GIMP icons. |
You should be able to find the source at gimp.org
|
I've added the magic wand icon. What shortcut key should I use for this? |
protected: | ||
void tilePositionChanged(const QPoint &tilePos); | ||
|
||
//void updateStatusInfo(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this commented line.
Done |
This implements the magic wand tool of #128