Skip to content
Draft

Image #4200

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion desktop/src/window/win/native_handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ unsafe extern "system" fn main_window_handle_message(hwnd: HWND, msg: u32, wpara
params.rgrc[0].bottom -= inset;
}

// Return 0 to to tell Windows to skip the default non-client area calculation and drawing.
// Return 0 to tell Windows to skip the default non-client area calculation and drawing.
return LRESULT(0);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,8 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageContext<'_>> for

// After import, `layer_node` is set to the root group. Apply the placement transform to it
// (skipped automatically when identity, so file-open with content at origin creates no Transform node).
modify_inputs.transform_set(placement_transform, TransformIn::Local, false);
modify_inputs.transform_set(placement_transform, TransformIn::Local, true);
responses.add(NodeGraphMessage::RunDocumentGraph);
}
}
}
Expand Down Expand Up @@ -617,8 +618,8 @@ fn import_usvg_node(
usvg::Node::Path(path) => {
import_usvg_path(modify_inputs, node, path, layer, graphite_gradient_stops);
}
usvg::Node::Image(_image) => {
warn!("Skip image");
usvg::Node::Image(image) => {
import_usvg_image(modify_inputs, node, image, layer);
}
usvg::Node::Text(text) => {
let font = Font::new(graphene_std::consts::DEFAULT_FONT_FAMILY.to_string(), graphene_std::consts::DEFAULT_FONT_STYLE.to_string());
Expand Down Expand Up @@ -668,8 +669,8 @@ fn import_usvg_node_inner(
import_usvg_path(modify_inputs, node, path, layer, graphite_gradient_stops);
0
}
usvg::Node::Image(_image) => {
warn!("Skip image");
usvg::Node::Image(image) => {
import_usvg_image(modify_inputs, node, image, layer);
0
}
usvg::Node::Text(text) => {
Expand Down Expand Up @@ -865,3 +866,35 @@ fn apply_usvg_fill(fill: &usvg::Fill, modify_inputs: &mut ModifyInputsContext, b
}
});
}

fn import_usvg_image(modify_inputs: &mut ModifyInputsContext, node: &usvg::Node, image: &usvg::Image, layer: LayerNodeIdentifier) {
let (image_data, format) = match image.kind() {
usvg::ImageKind::JPEG(data) => (data.as_slice(), ::image::ImageFormat::Jpeg),
usvg::ImageKind::PNG(data) => (data.as_slice(), ::image::ImageFormat::Png),
usvg::ImageKind::GIF(data) => (data.as_slice(), ::image::ImageFormat::Gif),
usvg::ImageKind::WEBP(data) => (data.as_slice(), ::image::ImageFormat::WebP),
_ => {
log::warn!("Unsupported SVG image format");
return;
}
};

let decoded_image = match ::image::load_from_memory_with_format(image_data, format) {
Ok(img) => img,
Err(e) => {
log::warn!("Failed to decode SVG image data: {:?}", e);
return;
}
};
let width = decoded_image.width();
let height = decoded_image.height();
Comment on lines +882 to +890
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Decoding the entire image using ::image::load_from_memory_with_format just to retrieve its dimensions is highly inefficient, especially for large embedded SVG images. Consider using ::image::Reader with into_dimensions() to parse only the image header and extract the width and height without decoding the full pixel data.

Suggested change
let decoded_image = match ::image::load_from_memory_with_format(image_data, format) {
Ok(img) => img,
Err(e) => {
log::warn!("Failed to decode SVG image data: {:?}", e);
return;
}
};
let width = decoded_image.width();
let height = decoded_image.height();
let (width, height) = match ::image::Reader::with_format(std::io::Cursor::new(image_data), format).into_dimensions() {
Ok(dim) => dim,
Err(e) => {
log::warn!("Failed to parse SVG image dimensions: {:?}", e);
return;
}
};

let transform_node_id = modify_inputs.insert_encoded_image_data(image_data.into(), layer);

let node_transform = usvg_transform(node.abs_transform());
let pixel_size = DVec2::new(width as f64, height as f64);
let final_transform = node_transform * DAffine2::from_scale(pixel_size);

if final_transform != DAffine2::IDENTITY {
transform_utils::update_transform(modify_inputs.network_interface, &transform_node_id, final_transform);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -299,28 +299,33 @@ impl<'a> ModifyInputsContext<'a> {
self.network_interface.move_node_to_chain_start(&color_value_id, layer, &[], self.import);
}

pub fn insert_image_data(&mut self, image: Image<Color>, layer: LayerNodeIdentifier) {
let transform = resolve_proto_node_type(graphene_std::transform_nodes::transform::IDENTIFIER)
.expect("Transform node does not exist")
.default_node_template();
pub fn insert_image_data(&mut self, image: Image<Color>, layer: LayerNodeIdentifier) -> NodeId {
self.insert_encoded_image_data(image.to_png().into(), layer)
}

pub fn insert_encoded_image_data(&mut self, data: std::sync::Arc<[u8]>, layer: LayerNodeIdentifier) -> NodeId {
let resource_id = ResourceId::new();
self.responses.add(ResourceMessage::StoreEmbedded {
resource_id,
data: image.to_png().into(),
});
self.responses.add_front(ResourceMessage::StoreEmbedded { resource_id, data });
self.insert_image_resource(resource_id, layer)
}

fn insert_image_resource(&mut self, resource_id: ResourceId, layer: LayerNodeIdentifier) -> NodeId {
let transform = resolve_proto_node_type(graphene_std::transform_nodes::transform::IDENTIFIER)
.expect("Transform node does not exist")
.default_node_template();
let image_node = resolve_proto_node_type(graphene_std::raster_nodes::std_nodes::image::IDENTIFIER)
.expect("Image node does not exist")
.node_template_input_override([Some(NodeInput::value(TaggedValue::Resource(resource_id), false))]);

let image_node_id = NodeId::new();
self.network_interface.insert_node(image_node_id, image_node, &[]);
self.network_interface.move_node_to_chain_start(&image_node_id, layer, &[], self.import);
let image_id = NodeId::new();
self.network_interface.insert_node(image_id, image_node, &[]);
self.network_interface.move_node_to_chain_start(&image_id, layer, &[], self.import);

let transform_id = NodeId::new();
self.network_interface.insert_node(transform_id, transform, &[]);
self.network_interface.move_node_to_chain_start(&transform_id, layer, &[], self.import);

transform_id
}

fn get_output_layer(&self) -> Option<LayerNodeIdentifier> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,170 @@
let hash = ResourceHash::from(data.as_ref());
self.registry.push_source_back(&resource_id, DataSource::Embedded);
self.registry.resolve(&resource_id, hash);
responses.add(ResourceStorageMessage::Store { data });
responses.add_front(ResourceStorageMessage::Store { data });
}
ResourceMessage::AddFont { resource_id, font } => {
let style = fonts.font_catalog.find_font_style_in_catalog(&font);
let style_name = style.map(|style| style.to_named_style()).unwrap_or_else(|| font.font_style.clone());
self.registry.push_source_back(&resource_id, DataSource::Embedded);
self.registry.push_source_back(
&resource_id,
DataSource::Font {
family: font.font_family,
style: Some(style_name),
},
);
responses.add(ResourceMessage::Resolve { resource_id });
}
ResourceMessage::ResolveAll => {
let unresolved_ids: Vec<ResourceId> = self.registry.unresolved().map(|info| info.id).collect();
for id in unresolved_ids {
if self.pending_resolves.contains(&id) {
continue;
}
responses.add(ResourceMessage::Resolve { resource_id: id });
}
}
ResourceMessage::Resolve { resource_id } => {
if self.pending_resolves.contains(&resource_id) {
log::warn!("Already pending resolve for {resource_id}; skipping");
return;
}
let Some(info) = self.registry.info(&resource_id) else {
log::error!("Resolve for {resource_id}: no registry entry");
return;
};
if info.hash.is_some() {
log::warn!("Resource {resource_id} already resolved");
return;
}

self.pending_resolves.insert(resource_id);

let font_catalog = fonts.font_catalog.clone();

let sources = info
.sources
.iter()
.map(|source| match source {
DataSource::Font { family, style } => {
let font = match style {
Some(style) => Font::new(family.clone(), style.clone()),
None => Font::new_with_default_style(family.clone()),
};
let hash = fonts.cached_hash(&font);
(source.clone(), hash)
}
source => (source.clone(), None),
})
.collect::<Vec<(DataSource, Option<ResourceHash>)>>();
Comment on lines +73 to +91
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When resolving a resource, if the resource is already cached locally (i.e., fonts.cached_hash(&font) returns Some(hash)), we can resolve it synchronously and return immediately. This avoids the overhead of spawning an asynchronous network request and dispatching a client call for already-cached resources.

				let sources = info
					.sources
					.iter()
					.map(|source| match source {
						DataSource::Font { family, style } => {
							let font = match style {
								Some(style) => Font::new(family.clone(), style.clone()),
								None => Font::new_with_default_style(family.clone()),
							};
							let hash = fonts.cached_hash(&font);
							(source.clone(), hash)
						}
						source => (source.clone(), None),
					})
					.collect::<Vec<(DataSource, Option<ResourceHash>)>>();

				if let Some((source, Some(hash))) = sources.iter().find(|(_, hash)| hash.is_some()) {
					responses.add(ResourceMessage::Resolved { resource_id, source: source.clone(), hash: *hash });
					return;
				}

				self.pending_resolves.insert(resource_id);

				let font_catalog = fonts.font_catalog.clone();


async fn resolve_to_message(document_id: DocumentId, resource_id: ResourceId, source: DataSource, url: Url, client: &Client) -> Option<Message> {
let result = client.fetch(url.clone()).await;
match result {
Some(data) => {
let hash = ResourceHash::from(data.as_ref());
Some(Message::Batched {
messages: Box::new([
PortfolioMessage::DocumentPassMessage {
document_id,
message: ResourceMessage::Resolved { resource_id, source, hash }.into(),
}
.into(),
ResourceStorageMessage::Store { data: Arc::from(data) }.into(),
]),
})
}
None => {
log::warn!("Failed to fetch resource {resource_id} from {url}");
None
}
}
}

responses.add(NetworkMessage::request(async move |client| {
let mut loaded_catalog = None;
let mut response: Option<Message> = None;
for (source, hash) in sources {
if let Some(hash) = hash {
response = Some(ResourceMessage::Resolved { resource_id, source, hash }.into());
break;
}

match &source {
DataSource::Embedded => continue,
DataSource::Url(url) => {
response = resolve_to_message(document_id, resource_id, source.clone(), url.clone(), &client).await;
}
DataSource::Font { family, style } => {
let font = match style {
Some(style) => Font::new(family.clone(), style.clone()),
None => Font::new_with_default_style(family.clone()),
};

if font_catalog.is_empty() && loaded_catalog.as_ref().is_none() {
loaded_catalog = FontCatalog::load_from_api(&client).await;
}

let url = loaded_catalog.as_ref().and_then(|catalog| catalog.download_url(&font)).or_else(|| font_catalog.download_url(&font));

if let Some(url) = url {
let Ok(url) = Url::parse(&url) else {
log::warn!("Invalid URL {url} for font resource {resource_id}");
continue;
};
response = resolve_to_message(document_id, resource_id, source.clone(), url, &client).await;
} else {
log::warn!("No download URL found for font resource {resource_id}");
}
}
}
if response.is_some() {
break;
}
}

let mut response = response.unwrap_or_else(|| {
log::error!("Resolve for {resource_id}: all sources exhausted");
PortfolioMessage::DocumentPassMessage {
document_id,
message: ResourceMessage::ResolveFailed { resource_id }.into(),
}
.into()
});

if let Some(catalog) = loaded_catalog.take() {
response = Message::Batched {
messages: Box::new([response, FontsMessage::CatalogLoaded { catalog }.into()]),
};
}

response
}))
}
ResourceMessage::Resolved { resource_id, source, hash } => {
self.pending_resolves.remove(&resource_id);
if self.registry.info(&resource_id).is_none() {
// Resource was removed from registry after the fetch started.
return;
}

self.registry.resolve(&resource_id, hash);

if let DataSource::Font { family, style } = source {
let font = match style {
Some(style) => Font::new(family, style),
None => Font::new_with_default_style(family),
};
responses.add(FontsMessage::ResourceResolved { font, hash });
}

responses.add(NodeGraphMessage::RunDocumentGraph);
}
ResourceMessage::ResolveFailed { resource_id } => {
self.pending_resolves.remove(&resource_id);
}
ResourceMessage::AddFont { resource_id, font } => {

Check failure on line 198 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 198 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern

Check failure on line 198 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 198 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern
let style = fonts.font_catalog.find_font_style_in_catalog(&font);
let style_name = style.map(|style| style.to_named_style()).unwrap_or_else(|| font.font_style.clone());
self.registry.push_source_back(&resource_id, DataSource::Embedded);
Expand All @@ -47,7 +208,7 @@
);
responses.add(ResourceMessage::Resolve { resource_id });
}
ResourceMessage::ResolveAll => {

Check failure on line 211 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 211 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern

Check failure on line 211 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 211 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern
let unresolved_ids: Vec<ResourceId> = self.registry.unresolved().map(|info| info.id).collect();
for id in unresolved_ids {
if self.pending_resolves.contains(&id) {
Expand All @@ -56,7 +217,7 @@
responses.add(ResourceMessage::Resolve { resource_id: id });
}
}
ResourceMessage::Resolve { resource_id } => {

Check failure on line 220 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 220 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern

Check failure on line 220 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 220 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern
if self.pending_resolves.contains(&resource_id) {
log::warn!("Already pending resolve for {resource_id}; skipping");
return;
Expand Down Expand Up @@ -173,7 +334,7 @@
response
}))
}
ResourceMessage::Resolved { resource_id, source, hash } => {

Check failure on line 337 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 337 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern

Check failure on line 337 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 337 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern
self.pending_resolves.remove(&resource_id);
if self.registry.info(&resource_id).is_none() {
// Resource was removed from registry after the fetch started.
Expand All @@ -192,7 +353,7 @@

responses.add(NodeGraphMessage::RunDocumentGraph);
}
ResourceMessage::ResolveFailed { resource_id } => {

Check failure on line 356 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 356 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern

Check failure on line 356 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / test

unreachable pattern

Check warning on line 356 in editor/src/messages/portfolio/document/resource/resource_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web

unreachable pattern
self.pending_resolves.remove(&resource_id);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ pub fn arc_outline(layer: Option<LayerNodeIdentifier>, document: &DocumentMessag
overlay_context.outline(subpath.iter(), viewport, None);
}

/// Check if the the cursor is inside the geometric star shape made by the Star node without any upstream node modifications
/// Check if the cursor is inside the geometric star shape made by the Star node without any upstream node modifications
pub fn inside_star(viewport: DAffine2, n: u32, radius1: f64, radius2: f64, mouse_position: DVec2) -> bool {
let mut paths = Vec::new();

Expand Down Expand Up @@ -520,7 +520,7 @@ pub fn inside_star(viewport: DAffine2, n: u32, radius1: f64, radius2: f64, mouse
winding != 0
}

/// Check if the the cursor is inside the geometric polygon shape made by the Polygon node without any upstream node modifications
/// Check if the cursor is inside the geometric polygon shape made by the Polygon node without any upstream node modifications
pub fn inside_polygon(viewport: DAffine2, n: u32, radius: f64, mouse_position: DVec2) -> bool {
let mut paths = Vec::new();

Expand Down
2 changes: 1 addition & 1 deletion editor/src/node_graph_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ impl NodeGraphExecutor {
graphene_std::raster::Image {
width: image.width,
height: image.height,
data: image.data.iter().map(|&c| SRGBA8::from(c)).collect(),
data: image.data.iter().map(|&c| SRGBA8::from(c.to_unassociated_alpha())).collect(),
base64_string: image.base64_string,
},
)
Expand Down
2 changes: 1 addition & 1 deletion node-graph/libraries/rendering/src/renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1797,7 +1797,7 @@ impl Render for List<Raster<GPU>> {
// Since colors and gradients are technically infinitely big, we have to implement
// workarounds for rendering them correctly in a way which still allows us
// to cache the intermediate render data (SVG string/Vello scene).
// For SVG, this is is achived by creating a truly giant rectangle.
// For SVG, this is achieved by creating a truly giant rectangle.
// For Vello, we create a layer with a placeholder transform which we
// later replace with the current viewport transform before each render.
impl Render for List<Color> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ fn target_active_square(active_list_levels: &[ActiveListLevel], rng: &mut impl F
return (level, active_square_index_in_level);
}

panic!("index_into_area couldn't be be mapped to a square in any level of the active lists");
panic!("index_into_area couldn't be mapped to a square in any level of the active lists");
}

fn point_not_covered_by_poisson_points(point: DVec2, diameter_squared: f64, points_grid: &AccelerationGrid) -> bool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn solve_spline_first_handle_open(points: &[DVec2]) -> Vec<DVec2> {

let mut c = vec![DVec2::new(1., 1.); len_points];

// 'd' is the the second point in a cubic bezier, which is what we solve for
// 'd' is the second point in a cubic bezier, which is what we solve for
let mut d = vec![DVec2::ZERO; len_points];

d[0] = DVec2::new(2. * points[1].x + points[0].x, 2. * points[1].y + points[0].y);
Expand Down
Loading