Skip to content

Add support for importing embedded SVG image elements#4200

Open
jsjgdh wants to merge 1 commit into
GraphiteEditor:masterfrom
jsjgdh:image
Open

Add support for importing embedded SVG image elements#4200
jsjgdh wants to merge 1 commit into
GraphiteEditor:masterfrom
jsjgdh:image

Conversation

@jsjgdh

@jsjgdh jsjgdh commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@jsjgdh jsjgdh closed this Jun 5, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request refactors the font and resource loading system by introducing a backend-driven asynchronous resource resolution system using a new network message handler and client. Text nodes are updated to use font resources via ResourceId instead of passing Font structs directly, and support is added for importing embedded SVG images. The review feedback suggests optimizing SVG image dimension extraction by reading only the image headers instead of decoding full pixel data, and resolving already-cached resources synchronously to avoid spawning redundant asynchronous network requests.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread editor/src/messages/portfolio/document/resource/resource_message_handler.rs Outdated
@jsjgdh jsjgdh reopened this Jun 7, 2026
@jsjgdh jsjgdh changed the title Image Add support for importing embedded SVG image elements Jun 8, 2026
@jsjgdh

jsjgdh commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@cubic-dev

@cubic-dev-ai

cubic-dev-ai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev

@jsjgdh I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

1 issue found across 11 files

Confidence score: 4/5

  • This PR appears safe to merge with minimal risk because the reported issue is a moderate performance inefficiency (5/10) rather than a correctness or data-loss bug.
  • In editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs, embedded image bytes are decoded once during import just to read dimensions and then decoded again later for rendering, which can add avoidable CPU overhead for image-heavy workflows.
  • Pay close attention to editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs - avoid redundant image decode work between import-time metadata extraction and render-time use.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@jsjgdh jsjgdh marked this pull request as ready for review June 8, 2026 20:35

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

1 issue found across 6 files

Confidence score: 3/5

  • There is a concrete user-facing risk in editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs: import_usvg_image ignores usvg::ImageKind::SVG, so embedded SVG <image> content can be silently dropped during import.
  • I’m scoring this as moderate merge risk because the issue is medium severity (6/10) with high confidence (9/10), and it affects import correctness rather than being purely internal or cosmetic.
  • Pay close attention to editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs - ensure SVG image kinds are handled so imported documents don’t lose embedded vector image content.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant