Fix fill tool on raster image temporarily breaking the graph (#2398)

* Fix fill tool on raster image temporarily breaks the graph

* Avoid vector filling raster layer via checking node category

* check raster image using input type instead

* Add additional check for TextureFrameTable

* Enable the ignore raster test

---------

Co-authored-by: hypercube <0hypercube@gmail.com>
This commit is contained in:
Ellen Gu 2025-03-09 15:18:42 -04:00 committed by GitHub
parent 56662339cc
commit 74b6abbb97
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 35 additions and 13 deletions

View file

@ -1655,7 +1655,7 @@ impl DocumentMessageHandler {
path.pop();
}
}
structure_section.push(space | 1 << 63);
structure_section.push(space | (1 << 63));
}
/// Serializes the layer structure into a condensed 1D structure.

View file

@ -1095,7 +1095,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
},
DocumentNodeDefinition {
identifier: "Image",
category: "",
category: "Raster",
node_template: NodeTemplate {
document_node: DocumentNode {
implementation: DocumentNodeImplementation::Network(NodeNetwork {

View file

@ -366,7 +366,7 @@ impl NodeNetworkInterface {
// If a chain node does not have a selected downstream layer, then set the position to absolute
let downstream_layer = self.downstream_layer(node_id, network_path);
if downstream_layer.map_or(true, |downstream_layer| new_ids.keys().all(|key| *key != downstream_layer.to_node())) {
if downstream_layer.is_none_or(|downstream_layer| new_ids.keys().all(|key| *key != downstream_layer.to_node())) {
let Some(position) = self.position(node_id, network_path) else {
log::error!("Could not get position in create_node_template");
return None;

View file

@ -5,6 +5,7 @@ use crate::messages::portfolio::document::utility_types::network_interface::{Flo
use crate::messages::prelude::*;
use bezier_rs::Subpath;
use graph_craft::concrete;
use graph_craft::document::{value::TaggedValue, NodeId, NodeInput};
use graphene_core::raster::image::ImageFrameTable;
use graphene_core::raster::BlendMode;
@ -417,4 +418,16 @@ impl<'a> NodeGraphLayer<'a> {
// TODO: Find a better way to accept a node input rather than using its index (which is quite unclear and fragile)
self.find_node_inputs(node_name)?.get(index)?.as_value()
}
/// Check if a layer is a raster layer
pub fn is_raster_layer(layer: LayerNodeIdentifier, network_interface: &mut NodeNetworkInterface) -> bool {
let layer_input_type = network_interface.input_type(&InputConnector::node(layer.to_node(), 1), &[]).0.nested_type();
if layer_input_type == concrete!(graphene_core::raster::image::ImageFrameTable<graphene_core::Color>)
|| layer_input_type == concrete!(graphene_core::application_io::TextureFrameTable)
|| layer_input_type == concrete!(graphene_std::RasterFrame)
{
return true;
}
false
}
}

View file

@ -89,7 +89,7 @@ impl AlignmentSnapper {
if let Some(point_on_x) = point_on_x {
let distance_to_snapped = point.document_point.distance(point_on_x);
let distance_to_align_target = point_on_x.distance(target_position);
if distance_to_snapped < tolerance && snap_x.as_ref().map_or(true, |point| distance_to_align_target < point.distance_to_align_target) {
if distance_to_snapped < tolerance && snap_x.as_ref().is_none_or(|point| distance_to_align_target < point.distance_to_align_target) {
snap_x = Some(SnappedPoint {
snapped_point_document: point_on_x,
source: point.source, // TODO(0Hypercube): map source
@ -108,7 +108,7 @@ impl AlignmentSnapper {
if let Some(point_on_y) = point_on_y {
let distance_to_snapped = point.document_point.distance(point_on_y);
let distance_to_align_target = point_on_y.distance(target_position);
if distance_to_snapped < tolerance && snap_y.as_ref().map_or(true, |point| distance_to_align_target < point.distance_to_align_target) {
if distance_to_snapped < tolerance && snap_y.as_ref().is_none_or(|point| distance_to_align_target < point.distance_to_align_target) {
snap_y = Some(SnappedPoint {
snapped_point_document: point_on_y,
source: point.source, // TODO(0Hypercube): map source

View file

@ -1,7 +1,6 @@
use super::tool_prelude::*;
use crate::messages::tool::common_functionality::graph_modification_utils::NodeGraphLayer;
use graphene_core::vector::style::Fill;
#[derive(Default)]
pub struct FillTool {
fsm_state: FillToolFsmState,
@ -87,6 +86,10 @@ impl Fsm for FillToolFsmState {
let Some(layer_identifier) = document.click(input) else {
return self;
};
// If the layer is a raster layer, don't fill it, wait till the flood fill tool is implemented
if NodeGraphLayer::is_raster_layer(layer_identifier, &mut document.network_interface) {
return self;
}
let fill = match color_event {
FillToolMessage::FillPrimaryColor => Fill::Solid(global_tool_data.primary_color),
FillToolMessage::FillSecondaryColor => Fill::Solid(global_tool_data.secondary_color),
@ -147,8 +150,6 @@ mod test_fill {
}
#[tokio::test]
// TODO: fix https://github.com/GraphiteEditor/Graphite/issues/2270
#[should_panic]
async fn ignore_raster() {
let mut editor = EditorTestUtils::create();
editor.new_document().await;

View file

@ -311,7 +311,7 @@ impl Fsm for FreehandToolFsmState {
}
fn extend_path_with_next_segment(tool_data: &mut FreehandToolData, position: DVec2, extend: bool, responses: &mut VecDeque<Message>) {
if !tool_data.end_point.map_or(true, |(last_pos, _)| position != last_pos) || !position.is_finite() {
if !tool_data.end_point.is_none_or(|(last_pos, _)| position != last_pos) || !position.is_finite() {
return;
}

View file

@ -364,7 +364,7 @@ impl Fsm for SplineToolFsmState {
return SplineToolFsmState::Ready;
};
tool_data.next_point = tool_data.snapped_point(document, input).snapped_point_document;
if tool_data.points.last().map_or(true, |last_pos| last_pos.1.distance(tool_data.next_point) > DRAG_THRESHOLD) {
if tool_data.points.last().is_none_or(|last_pos| last_pos.1.distance(tool_data.next_point) > DRAG_THRESHOLD) {
let preview_point = tool_data.preview_point;
extend_spline(tool_data, false, responses);
tool_data.preview_point = preview_point;