Skip to content

Copy paste with resources#4294

Closed
Keavon wants to merge 1 commit into
masterfrom
copy-paste-with-resources
Closed

Copy paste with resources#4294
Keavon wants to merge 1 commit into
masterfrom
copy-paste-with-resources

Conversation

@Keavon

@Keavon Keavon commented Jun 30, 2026

Copy link
Copy Markdown
Member

Fixes the bug that copy-pasting a Text layer into a different document results in intermittent render errors and console errors.

@Keavon Keavon requested a review from timon-schelling June 30, 2026 08:57

@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 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.

Comment on lines +2265 to +2271
for input in &node.inputs {
if let NodeInput::Value { tagged_value, .. } = input
&& let TaggedValue::Resource(id) = &**tagged_value
{
out.insert(*id);
}
}

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.

high

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);
			}
		}
	}

Comment on lines +1064 to +1067
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);
}

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

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.

Suggested change
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));
}

Comment on lines +1089 to +1093
if !clipboard_resources.is_empty()
&& let Some(document_id) = self.active_document_id
{
responses.add(PortfolioMessage::ResolveDocumentResources { document_id });
}

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

Use the has_resources boolean flag defined at the top of the match arm instead of checking !clipboard_resources.is_empty() on the collected vector.

				if has_resources && let Some(document_id) = self.active_document_id {
					responses.add(PortfolioMessage::ResolveDocumentResources { document_id });
				}

@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 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(),

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.

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>

@timon-schelling

Copy link
Copy Markdown
Member

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

@timon-schelling

Copy link
Copy Markdown
Member

We should also take this as a opportunity to move all the clipboard code into a separate message handler.

@timon-schelling timon-schelling marked this pull request as draft June 30, 2026 12:08
@Keavon Keavon closed this Jul 1, 2026
@Keavon Keavon deleted the copy-paste-with-resources branch July 1, 2026 10:33
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.

2 participants