-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: optimize memory usage for image handling in webview #7556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
83ca364
253deb8
1a34943
eccf51e
0365f9b
21d174e
6f6aead
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -658,9 +658,17 @@ export class ClineProvider | |
| setTtsSpeed(ttsSpeed ?? 1) | ||
| }) | ||
|
|
||
| // Set up webview options with proper resource roots | ||
| const resourceRoots = [this.contextProxy.extensionUri] | ||
|
|
||
| // Add workspace folders to allow access to workspace files | ||
| if (vscode.workspace.workspaceFolders) { | ||
| resourceRoots.push(...vscode.workspace.workspaceFolders.map((folder) => folder.uri)) | ||
| } | ||
|
|
||
| webviewView.webview.options = { | ||
| enableScripts: true, | ||
| localResourceRoots: [this.contextProxy.extensionUri], | ||
| localResourceRoots: resourceRoots, | ||
| } | ||
|
|
||
| webviewView.webview.html = | ||
|
|
@@ -2399,6 +2407,32 @@ export class ClineProvider | |
| }) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Convert a file path to a webview-accessible URI | ||
| * This method safely converts file paths to URIs that can be loaded in the webview | ||
| * | ||
| * @param filePath - The absolute file path to convert | ||
| * @returns The webview URI string, or the original file URI if conversion fails | ||
| */ | ||
| public convertToWebviewUri(filePath: string): string { | ||
| try { | ||
| const fileUri = vscode.Uri.file(filePath) | ||
|
|
||
| // Check if we have a webview available | ||
| if (this.view?.webview) { | ||
| const webviewUri = this.view.webview.asWebviewUri(fileUri) | ||
| return webviewUri.toString() | ||
| } | ||
|
|
||
| // Fallback to file URI if no webview available | ||
| return fileUri.toString() | ||
| } catch (error) { | ||
| console.error("Failed to convert to webview URI:", error) | ||
|
||
| // Return file URI as fallback | ||
| return vscode.Uri.file(filePath).toString() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| class OrganizationAllowListViolationError extends Error { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1159,6 +1159,14 @@ export const ChatRowContent = ({ | |||||
| return <CodebaseSearchResultsDisplay results={results} /> | ||||||
| case "user_edit_todos": | ||||||
| return <UpdateTodoListToolBlock userEdited onChange={() => {}} /> | ||||||
| case "image": | ||||||
| // Parse the JSON to get imageUri and imagePath | ||||||
| const imageInfo = JSON.parse(message.text || "{}") | ||||||
|
||||||
| const imageInfo = JSON.parse(message.text || "{}") | |
| const imageInfo = safeJsonParse<any>(message.text || "{}") |
This comment was generated because it violated a code review rule: irule_PTI8rjtnhwrWq6jS.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a proper type definition for the parsed image info instead of using a generic object:
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,42 @@ import React from "react" | |
| import { ImageViewer } from "./ImageViewer" | ||
|
|
||
| interface ImageBlockProps { | ||
| imageData: string | ||
| path?: string | ||
| // For new image generation tool format (preferred) | ||
|
||
| imageUri?: string // The webview-accessible URI for rendering | ||
| imagePath?: string // The actual file path for display and opening | ||
|
|
||
| // For backward compatibility with Mermaid diagrams and old format | ||
| imageData?: string // Base64 data or regular URL (legacy) | ||
| path?: string // Optional path for Mermaid diagrams (legacy) | ||
| } | ||
|
|
||
| export default function ImageBlock({ imageData, path }: ImageBlockProps) { | ||
| export default function ImageBlock({ imageUri, imagePath, imageData, path }: ImageBlockProps) { | ||
| // Determine which props to use based on what's provided | ||
| let finalImageUri: string | ||
| let finalImagePath: string | undefined | ||
|
|
||
| if (imageUri) { | ||
| // New format: explicit imageUri and imagePath | ||
| finalImageUri = imageUri | ||
| finalImagePath = imagePath | ||
| } else if (imageData) { | ||
| // Legacy format: use imageData as direct URI (for Mermaid diagrams) | ||
| finalImageUri = imageData | ||
| finalImagePath = path | ||
| } else { | ||
| // No valid image data provided | ||
| console.error("ImageBlock: No valid image data provided") | ||
| return null | ||
| } | ||
|
|
||
| return ( | ||
| <div className="my-2"> | ||
| <ImageViewer imageData={imageData} path={path} alt="AI Generated Image" showControls={true} /> | ||
| <ImageViewer | ||
| imageUri={finalImageUri} | ||
| imagePath={finalImagePath} | ||
| alt="AI Generated Image" | ||
| showControls={true} | ||
| /> | ||
| </div> | ||
| ) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this import still needed? It appears to be unused after the refactoring: