Implement fill tool #254
Merged
Implement fill tool #254
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TrueDoctor
reviewed
Jul 13, 2021
Might as well! |
0HyperCube
previously requested changes
Jul 13, 2021
TrueDoctor
approved these changes
Jul 14, 2021
| @@ -56,6 +56,24 @@ impl PathStyle { | |||
| pub fn new(stroke: Option<Stroke>, fill: Option<Fill>) -> Self { | |||
| Self { stroke, fill } | |||
| } | |||
| pub fn get_fill(&self) -> Option<Fill> { | |||
There was a problem hiding this comment.
please remove the get wherever you see it: https://rust-lang.github.io/api-guidelines/naming.html
our codebase is not yet consistent in that regard but it is something we want to improve on
TrueDoctor
dismissed
0HyperCube’s stale review
It seems to me, as if all points have been addressed
|
Alright, feel free to merge once the ci is done |
Keavon
pushed a commit
that referenced
this pull request
Jun 16, 2022
* Implement fill tool * Add fill tool shortcut * Add getters and setters to styles * Make fill tool act on the topmost layer clicked * Refactor fill operation * Refactor and unify selection tolerance * Add mark_as_dirty function * Fix getter names
Keavon
pushed a commit
that referenced
this pull request
Jul 30, 2023
* Implement fill tool * Add fill tool shortcut * Add getters and setters to styles * Make fill tool act on the topmost layer clicked * Refactor fill operation * Refactor and unify selection tolerance * Add mark_as_dirty function * Fix getter names
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #152
set_fillmethod toFillFillandStrokefor consistency?fillwith a given colorMouseDown+Lmbto the Fill tool#[child]to thetool_message_handlerFillclick handler, generating aFillLayeroperation with the primary color, targeting a clicked layer (if any)The biggest caveat at the moment is the ad hoc click detection. I'm using
intersects_quad_rootwith a small rectangle centered on the mouse. This works (though it gives an extra pixel of padding to the click), but I imagineintersects_pointandintersects_point_rootfunctions should be implemented in the future (I can open an issue for that if we agree it would be preferred).Also, when multiple layers are returned by
intersects_quad_root, I am using the first of them. This seems to correspond to the visually lowest layer; should it instead use the last item returned?This change is