-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix: check if path tool is interacting with raster layer to prevent crashing. #3182
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
base: master
Are you sure you want to change the base?
Fix: check if path tool is interacting with raster layer to prevent crashing. #3182
Conversation
581ff33 to
e8a0e36
Compare
|
!build |
|
|
Video of the fix: Graphite.path.tool.bugfix.mov |
|
Please let me know if the video above captures the expected behavior. |
|
This does not remove anchors from the corners of the image. All it does is break the Path tool's ability to select stuff, either a box selection or even clicking on other valid points if multiple layers (including actual vector layers) are selected. Please give it another shot so it addresses the requested change (first sentence of this reply). |
|
So there shouldn't be any anchors displayed on the image at all when the path tool is selected? The path tool still works on the spline as shown, it's just ignoring the raster image. |
|
I added a check before showing the anchors/handles. Please let me know if this PR is in-line with what you expected now. |
|
Okay, I simplified the code changes. I think this PR should fulfill the requirements now. |
|
|
@Keavon Should we merge this? |
124235a to
a42cad8
Compare
| #[message_handler_data] | ||
| impl<'a> MessageHandler<ToolMessage, &mut ToolActionMessageContext<'a>> for PathTool { | ||
| fn process_message(&mut self, message: ToolMessage, responses: &mut VecDeque<Message>, context: &mut ToolActionMessageContext<'a>) { | ||
| let target_layers: Vec<LayerNodeIdentifier> = context.document.network_interface.selected_nodes().selected_layers(context.document.metadata()).collect(); |
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 can be filtered before collecting
adamgerhant
left a comment
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 is not an optimal implementation since it requires an entire document click test every time a message is sent to the Path tool, including overlays and such. Ideally the check should be done in the already existing selection logic. The PathToolMessage::SelectionChanged message handling might be a good place to add this filter.
Resolves #2687
Added a check that returns
PathToolFsmState::Readyif the path tool is used on a raster layer.