fork/Graphite
1
0
Fork 0
mirror of https://github.com/GraphiteEditor/Graphite.git synced 2026-09-21 18:38:11 +08:00
Code Issues Packages Projects Releases Wiki Activity

Restructure node graph execution to be safer (#1277)

Browse Source 
* Reorganize file structure

* Remove all unsafe code

* Add testcase for debugging ub

* Convert into proper test with fail condition

* General cleanup

* Fix tests

* Add feature guard for deallocation

* Use raw pointer for storing values to avoid violating aliasing rules

* Add comment explaining the disabling of simd128

* Fix brush node

* Fix formatting
This commit is contained in:
Dennis Kobert
2023-06-03 01:18:44 +02:00
committed by Keavon Chambers
parent 5558deba5e
commit 26473a8002
29 changed files with 363 additions and 299 deletions
Download Patch File Download Diff File Expand all files Collapse all files
editor/src/application.rs
+54 -4
View File
File diff suppressed because one or more lines are too long
editor/src/messages/message.rs
-2
View File
@@ -3,8 +3,6 @@ use crate::messages::prelude::*;
use graphite_proc_macros::*;
use serde::{Deserialize, Serialize};
use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};
#[remain::sorted]
#[impl_message]
editor/src/messages/portfolio/document/document_message_handler.rs
-4
View File
@@ -1001,10 +1001,6 @@ impl DocumentMessageHandler {
// Calculate the size of the region to be exported and generate an SVG of the artwork below this layer within that region
let transform = self.document_legacy.multiply_transforms(&layer_path).unwrap();
let size = DVec2::new(transform.transform_vector2(DVec2::new(1., 0.)).length(), transform.transform_vector2(DVec2::new(0., 1.)).length());
// TODO: Fix this hack
// This is a hack to prevent the compiler from optimizing out the size calculation which likely is due
// to undefined behavior. THIS IS NOT A FIX.
log::trace!("size: {:?}", size);
let svg = self.render_document(size, transform.inverse(), persistent_data, DocumentRenderMode::OnlyBelowLayerInFolder(&layer_path));
self.restore_document_transform(old_transforms);
editor/src/messages/portfolio/document/node_graph/graph_operation_message_handler.rs
+1 -5
View File
@@ -224,11 +224,7 @@ impl<'a> ModifyInputsContext<'a> {
fn brush_modify(&mut self, strokes: Vec<BrushStroke>) {
self.modify_inputs("Brush", false, |inputs| {
if matches!(inputs[0], NodeInput::Node { .. }) {
inputs[1] = core::mem::replace(&mut inputs[0], NodeInput::value(TaggedValue::None, false));
}
inputs[0] = NodeInput::value(TaggedValue::None, false);
inputs[3] = NodeInput::value(TaggedValue::BrushStrokes(strokes), false);
inputs[2] = NodeInput::value(TaggedValue::BrushStrokes(strokes), false);
});
}
}
editor/src/messages/portfolio/document/node_graph/node_graph_message_handler/document_node_types.rs
+1 -2
View File
@@ -669,9 +669,8 @@ fn static_nodes() -> Vec<DocumentNodeType> {
DocumentNodeType {
name: "Brush",
category: "Brush",
identifier: NodeImplementation::proto("graphene_std::brush::BrushNode"),
identifier: NodeImplementation::proto("graphene_std::brush::BrushNode<_, _>"),
inputs: vec![
DocumentInputType::value("None", TaggedValue::None, false),
DocumentInputType::value("Background", TaggedValue::ImageFrame(ImageFrame::empty()), true),
DocumentInputType::value("Bounds", TaggedValue::ImageFrame(ImageFrame::empty()), true),
DocumentInputType::value("Trace", TaggedValue::BrushStrokes(Vec::new()), false),
editor/src/messages/tool/tool_messages/brush_tool.rs
+6 -7
View File
@@ -14,7 +14,7 @@ use crate::messages::tool::utility_types::{HintData, HintGroup, HintInfo};
use document_legacy::layers::layer_layer::CachedOutputData;
use document_legacy::LayerId;
use graph_craft::document::value::TaggedValue;
use graph_craft::document::{NodeId, NodeInput, NodeNetwork};
use graph_craft::document::{NodeInput, NodeNetwork};
use graphene_core::raster::{BlendMode, ImageFrame};
use graphene_core::vector::brush_stroke::{BrushInputSample, BrushStroke, BrushStyle};
use graphene_core::Color;
@@ -300,7 +300,6 @@ impl ToolTransition for BrushTool {
struct BrushToolData {
strokes: Vec<BrushStroke>,
layer_path: Vec<LayerId>,
node_path: Vec<NodeId>,
transform: DAffine2,
}
@@ -315,7 +314,7 @@ impl BrushToolData {
let network = &layer.network;
for (node, _node_id) in network.primary_flow() {
if node.name == "Brush" {
let points_input = node.inputs.get(3)?;
let points_input = node.inputs.get(2)?;
let NodeInput::Value { tagged_value: TaggedValue::BrushStrokes(strokes), .. } = points_input else {
continue;
};
@@ -332,7 +331,7 @@ impl BrushToolData {
matches!(layer.cached_output_data, CachedOutputData::BlobURL(_) | CachedOutputData::SurfaceId(_)).then_some(&self.layer_path)
}
fn update_strokes(&self, brush_options: &BrushOptions, responses: &mut VecDeque<Message>) {
fn update_strokes(&self, responses: &mut VecDeque<Message>) {
let layer = self.layer_path.clone();
let strokes = self.strokes.clone();
responses.add(GraphOperationMessage::Brush { layer, strokes });
@@ -394,7 +393,7 @@ impl Fsm for BrushToolFsmState {
if new_layer {
add_brush_render(tool_options, tool_data, responses);
}
tool_data.update_strokes(tool_options, responses);
tool_data.update_strokes(responses);
BrushToolFsmState::Drawing
}
@@ -403,7 +402,7 @@ impl Fsm for BrushToolFsmState {
if let Some(stroke) = tool_data.strokes.last_mut() {
stroke.trace.push(BrushInputSample { position: layer_position })
}
tool_data.update_strokes(tool_options, responses);
tool_data.update_strokes(responses);
BrushToolFsmState::Drawing
}
@@ -448,7 +447,7 @@ impl Fsm for BrushToolFsmState {
}
}
fn add_brush_render(tool_options: &BrushOptions, data: &BrushToolData, responses: &mut VecDeque<Message>) {
fn add_brush_render(_tool_options: &BrushOptions, data: &BrushToolData, responses: &mut VecDeque<Message>) {
let mut network = NodeNetwork::default();
let output_node = network.push_output_node();
if let Some(node) = network.nodes.get_mut(&output_node) {
editor/src/node_graph_executor.rs
+21 -10
View File
@@ -9,7 +9,7 @@ use document_legacy::{LayerId, Operation};
use graph_craft::document::value::TaggedValue;
use graph_craft::document::{generate_uuid, DocumentNodeImplementation, NodeId, NodeNetwork};
use graph_craft::executor::Compiler;
use graph_craft::graphene_compiler::Compiler;
use graph_craft::{concrete, Type, TypeDescriptor};
use graphene_core::application_io::ApplicationIo;
use graphene_core::raster::{Image, ImageFrame};
@@ -19,7 +19,7 @@ use graphene_core::vector::style::ViewMode;
use graphene_core::wasm_application_io::WasmApplicationIo;
use graphene_core::{Color, EditorApi, SurfaceFrame, SurfaceId};
use interpreted_executor::executor::DynamicExecutor;
use interpreted_executor::dynamic_executor::DynamicExecutor;
use glam::{DAffine2, DVec2};
use std::borrow::Cow;
@@ -147,7 +147,7 @@ impl NodeRuntime {
return Err(e);
}
use graph_craft::executor::Executor;
use graph_craft::graphene_compiler::Executor;
let result = match self.executor.input_type() {
Some(t) if t == concrete!(EditorApi) => (&self.executor).execute(editor_api).await.map_err(|e| e.to_string()),
@@ -155,7 +155,7 @@ impl NodeRuntime {
_ => Err("Invalid input type".to_string()),
}?;
if let TaggedValue::SurfaceFrame(SurfaceFrame { surface_id, transform }) = result {
if let TaggedValue::SurfaceFrame(SurfaceFrame { surface_id, transform: _ }) = result {
let old_id = self.canvas_cache.insert(path.to_vec(), surface_id);
if let Some(old_id) = old_id {
if old_id != surface_id {
@@ -206,6 +206,19 @@ impl NodeRuntime {
}
}
}
pub fn introspect_node(path: &[NodeId]) -> Option<Arc<dyn std::any::Any>> {
NODE_RUNTIME
.try_with(|runtime| {
let runtime = runtime.try_borrow();
if let Ok(ref runtime) = runtime {
if let Some(ref mut runtime) = runtime.as_ref() {
return runtime.executor.introspect(path).flatten();
}
}
None
})
.unwrap_or(None)
}
pub async fn run_node_graph() {
let result = NODE_RUNTIME.try_with(|runtime| {
@@ -226,7 +239,6 @@ pub async fn run_node_graph() {
#[derive(Debug)]
pub struct NodeGraphExecutor {
pub(crate) executor: DynamicExecutor,
sender: Sender<NodeRuntimeMessage>,
receiver: Receiver<GenerationResponse>,
// TODO: This is a memory leak since layers are never removed
@@ -250,7 +262,6 @@ impl Default for NodeGraphExecutor {
});
Self {
executor: Default::default(),
futures: Default::default(),
sender: request_sender,
receiver: response_reciever,
@@ -275,12 +286,12 @@ impl NodeGraphExecutor {
generation_id
}
pub fn update_font_cache(&self, font_cache: FontCache) {
self.sender.send(NodeRuntimeMessage::FontCacheUpdate(font_cache)).expect("Failed to send font cache update");
pub fn introspect_node(&self, path: &[NodeId]) -> Option<Arc<dyn std::any::Any>> {
introspect_node(path)
}
pub fn introspect_node(&self, path: &[NodeId]) -> Option<Arc<dyn std::any::Any>> {
self.executor.introspect(path).flatten()
pub fn update_font_cache(&self, font_cache: FontCache) {
self.sender.send(NodeRuntimeMessage::FontCacheUpdate(font_cache)).expect("Failed to send font cache update");
}
pub fn previous_output_type(&self, path: &[LayerId]) -> Option<Type> {