Copy paste with resources#4294
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to preserve and re-register document resources during copy-paste operations across documents. It adds a ClipboardResource struct to carry resource registry snapshots in the clipboard and implements logic to collect these resources during copy operations and register them upon pasting. The review feedback suggests several improvements: recursively traversing TaggedValue::DocumentNode to ensure nested resources are collected, and optimizing the paste logic in PasteIntoFolder to avoid unnecessary allocations and clones by using lazy iterators and a boolean flag.
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.
| for input in &node.inputs { | ||
| if let NodeInput::Value { tagged_value, .. } = input | ||
| && let TaggedValue::Resource(id) = &**tagged_value | ||
| { | ||
| out.insert(*id); | ||
| } | ||
| } |
There was a problem hiding this comment.
Extracted or cloned nodes can be wrapped inside a TaggedValue::DocumentNode variant. Recursively traversing TaggedValue::DocumentNode ensures that resources nested inside these nodes are also collected and registered correctly during copy-paste operations.
for input in &node.inputs {
if let NodeInput::Value { tagged_value, .. } = input {
if let TaggedValue::Resource(id) = &**tagged_value {
out.insert(*id);
} else if let TaggedValue::DocumentNode(inner_node) = &**tagged_value {
collect_document_node_resources(inner_node, out);
}
}
}| let clipboard_resources: Vec<ClipboardResource> = self.copy_buffer[clipboard as usize].iter().flat_map(|entry| entry.resources.iter().cloned()).collect(); | ||
| if let Some(document) = self.active_document_mut() { | ||
| register_clipboard_resources(document, &clipboard_resources); | ||
| } |
There was a problem hiding this comment.
Instead of collecting and cloning all clipboard resources into a temporary Vec, we can check if any resources exist using .any() and pass a lazy iterator to register_clipboard_resources. This avoids unnecessary allocations and clones, matching the pattern used in PasteSerializedData.
| let clipboard_resources: Vec<ClipboardResource> = self.copy_buffer[clipboard as usize].iter().flat_map(|entry| entry.resources.iter().cloned()).collect(); | |
| if let Some(document) = self.active_document_mut() { | |
| register_clipboard_resources(document, &clipboard_resources); | |
| } | |
| let has_resources = self.copy_buffer[clipboard as usize].iter().any(|entry| !entry.resources.is_empty()); | |
| if has_resources && let Some(document) = self.active_document_mut() { | |
| register_clipboard_resources(document, self.copy_buffer[clipboard as usize].iter().flat_map(|entry| &entry.resources)); | |
| } |
| if !clipboard_resources.is_empty() | ||
| && let Some(document_id) = self.active_document_id | ||
| { | ||
| responses.add(PortfolioMessage::ResolveDocumentResources { document_id }); | ||
| } |
There was a problem hiding this comment.
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 3/5
- In
editor/src/messages/portfolio/portfolio_message_handler.rs, clipboard snapshots for device resources appear to omit embedded bytes, so embedded-only images/fonts may paste as unresolved registry entries and fail to render in a new session. Before merging, include embedded payload data in the snapshot (or avoid marking those resources resolvable across sessions) and verify copy/paste across restart boundaries.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/messages/portfolio/portfolio_message_handler.rs">
<violation number="1" location="editor/src/messages/portfolio/portfolio_message_handler.rs:337">
P1: Device clipboard resource snapshots omit embedded resource bytes, so embedded-only images/fonts can paste as registry entries that cannot render in another session. Include embedded payloads or avoid marking them resolved unless bytes are present in target storage.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| Some(ClipboardResource { | ||
| id, | ||
| sources: info.sources.to_vec(), | ||
| hash: info.hash.copied(), |
There was a problem hiding this comment.
P1: Device clipboard resource snapshots omit embedded resource bytes, so embedded-only images/fonts can paste as registry entries that cannot render in another session. Include embedded payloads or avoid marking them resolved unless bytes are present in target storage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/portfolio_message_handler.rs, line 337:
<comment>Device clipboard resource snapshots omit embedded resource bytes, so embedded-only images/fonts can paste as registry entries that cannot render in another session. Include embedded payloads or avoid marking them resolved unless bytes are present in target storage.</comment>
<file context>
@@ -319,12 +320,32 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
+ Some(ClipboardResource {
+ id,
+ sources: info.sources.to_vec(),
+ hash: info.hash.copied(),
+ })
+ })
</file context>
|
This would still miss the content making for example images not work. I suggest that we instead make the content of a clipboard: // string format "graphite: <serialized GraphiteClipboardContent>"
type GraphiteClipboardContent = Vec<GraphiteClipboardItem>;
enum GraphiteClipboardItem {
Layers(...),
...
Resource {
id: ResourceId,
hash: Option<ResourceHash>,
sources: Vec<DataSource>,
data: Option<ResourceData>,
}
}
type ResourceData = Vec<u8>; // serialized as base64 string, maybe with compression |
|
We should also take this as a opportunity to move all the clipboard code into a separate message handler. |
Fixes the bug that copy-pasting a Text layer into a different document results in intermittent render errors and console errors.