add thumbnail for 3d generation #8129
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dosubot
bot
added
the
size:L
🎭 Playwright Tests:
|
Contributor
📝 WalkthroughWalkthroughAdds automatic thumbnail capture/upload for 3D models: a new Load3d.captureThumbnail method, Load3dUtils helpers to check/generate/upload thumbnails, triggers to run generation after model load (useLoad3d and saveMesh), and UI changes to show PNG thumbnails with a fallback placeholder. Changes
Sequence Diagram(s)sequenceDiagram
participant Node as Model Node
participant Composable as useLoad3d / saveMesh hook
participant Load3d as Load3d Instance
participant Utils as Load3dUtils
participant API as Backend API
Node->>Composable: modelLoadingEnd(modelPath, isPreview)
Composable->>Utils: generateThumbnailIfNeeded(load3d, modelPath, folderType)
Utils->>API: HEAD /<subfolder>/<filename>.png
API-->>Utils: 200/404
alt not exists
Utils->>Load3d: captureThumbnail(256,256)
activate Load3d
Load3d->>Load3d: save camera/grid state
Load3d->>Load3d: position camera & capture scene -> imageData
Load3d->>Load3d: restore camera/grid state
deactivate Load3d
Utils->>API: POST /upload/image (FormData with image, subfolder, type)
API-->>Utils: 200 OK
else exists
Utils-->>Composable: skip upload
end
Utils-->>Composable: done
Possibly related PRs
Suggested reviewers
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/18/2026, 03:14:18 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
coderabbitai
bot
requested review from
KarryCharon,
PabloWiedemann,
Yorha4D and
shinshin86
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 18.1 kB (baseline 18.1 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 878 kB (baseline 878 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 66.6 kB (baseline 66.6 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 385 kB (baseline 385 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 8 added / 8 removed User & Accounts — 4.22 kB (baseline 4.22 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.85 kB (baseline 2.85 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 26.3 kB (baseline 26.3 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.75 MB (baseline 2.75 MB) • 🔴 +2.07 kBStores, services, APIs, and repositories
Status: 7 added / 7 removed Utilities & Hooks — 16.6 kB (baseline 16.6 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 4 added / 4 removed Vendor & Third-Party — 9.09 MB (baseline 9.09 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 5.91 MB (baseline 5.91 MB) • 🔴 +838 BBundles that do not match a named category
Status: 26 added / 26 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/extensions/core/load3d/Load3dUtils.ts`:
- Around line 144-169: The uploadThumbnail function currently assumes imageData
is a valid data URL and will quietly fail on invalid input; add a basic
validation at the start of uploadThumbnail (e.g., check imageData is a non-empty
string and matches a data URL pattern like /^data:image\/[a-zA-Z]+;base64,/) and
if validation fails, log a clear warning via console.warn or processLogger and
return false before calling fetch; keep the rest of the function (creating
blob/File, FormData, api.fetchApi) unchanged so callers get an immediate,
predictable false for bad input rather than a fetch error.
In `@src/extensions/core/saveMesh.ts`:
- Around line 21-47: The duplicate thumbnail logic in
generateThumbnailForSaveMesh should be extracted into a shared static method on
Load3dUtils (e.g., generateThumbnailIfNeeded) that uses
Load3dUtils.splitFilePath, .getThumbnailFilename, .fileExists and
.uploadThumbnail and calls load3d.captureThumbnail; replace the body of
generateThumbnailForSaveMesh with a call to
Load3dUtils.generateThumbnailIfNeeded(load3d, modelPath, folderType) and update
the other duplicate in useLoad3d.ts to call the same utility, and add the
corresponding import for Load3dUtils where needed; preserve the try/catch and
console.warn behavior inside the new utility so callers remain simple.
In `@src/platform/assets/components/Media3DTop.vue`:
- Around line 29-34: The thumbnailError ref isn't reset when the asset prop
changes, so a previous load failure keeps showing the placeholder; update the
component to watch the asset (or thumbnailSrc) and reset thumbnailError to false
when it changes (use watch or watchEffect to observe asset or thumbnailSrc and
set thumbnailError.value = false), ensuring you import the appropriate Vue
watch/watchEffect symbol; reference the thumbnailError ref, the thumbnailSrc
computed, and the asset prop when adding the watcher.
| } | ||
| } | ||
|
|
||
| static async uploadThumbnail( |
Contributor
There was a problem hiding this comment.
Should we re-use the helpers that already exist -- e.g., the one that is 100 lines above this
Or this
Not really for this PR, but after looking, we seem to have a lot of duplication across the codebase for the upload endpoint usage:
- src/composables/node/useNodeImageUpload.ts:20-49 (most generic implementation)
- src/composables/maskeditor/useMaskEditorSaver.ts:266-299
- src/components/common/BackgroundImageUpload.vue:55-74
- src/extensions/core/webcamCapture.ts
- src/extensions/core/uploadAudio.ts:28-73
- src/services/audioService.ts
- src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue
Contributor
There was a problem hiding this comment.
@coderabbitai make an Issue to centralize all the upload helpers listed above into a singlar shared util or service that these modules then import from. Allows easier refactoring later whenever we adjust the upload logic or parameters.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
@christian-byrne, I'll create an issue to track this refactoring work.
🏁 Script executed:
#!/bin/bash
# Create GitHub issue to centralize upload helper functions
gh issue create \
--title "Centralize upload helper functions into shared utility" \
--body "## Context
Multiple upload helper implementations are duplicated across the codebase, making it difficult to maintain and refactor upload logic consistently.
## Locations with duplicated upload functionality
- \`src/composables/node/useNodeImageUpload.ts:20-49\` (most generic implementation)
- \`src/composables/maskeditor/useMaskEditorSaver.ts:266-299\`
- \`src/components/common/BackgroundImageUpload.vue:55-74\`
- \`src/extensions/core/webcamCapture.ts\`
- \`src/extensions/core/uploadAudio.ts:28-73\`
- \`src/services/audioService.ts\`
- \`src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue\`
## Proposal
Centralize these implementations into a single shared utility or service that all modules can import from. This will:
- Reduce code duplication
- Make future changes to upload logic easier to maintain
- Ensure consistent behavior across all upload operations
- Simplify testing and error handling
## References
- Identified in PR `#8129`: https://github.com/Comfy-Org/ComfyUI_frontend/pull/8129
- Comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/8129#discussion_r2701437891
- Requested by: `@christian-byrne`" \
--assignee christian-byrne
Length of output: 57
@christian-byrne, I've created issue #8130 to track the centralization of upload helper functions. The issue includes all the file locations you identified, the benefits of consolidating them, and references back to this PR and comment.
🧬 Code Graph Analysis Results
File: src/scripts/api.ts (lines 1304-1304)
export const api = new ComfyApi()
File: src/scripts/app.ts (lines 1746-1746)
export const app = new ComfyApp()
File: src/platform/updates/common/toastStore.ts (lines 8-39)
export const useToastStore = defineStore('toast', () => {
const messagesToAdd = ref<ToastMessageOptions[]>([])
const messagesToRemove = ref<ToastMessageOptions[]>([])
const removeAllRequested = ref(false)
function add(message: ToastMessageOptions) {
messagesToAdd.value = [...messagesToAdd.value, message]
}
function remove(message: ToastMessageOptions) {
messagesToRemove.value = [...messagesToRemove.value, message]
}
function removeAll() {
removeAllRequested.value = true
}
function addAlert(message: string) {
add({ severity: 'warn', summary: 'Alert', detail: message })
}
return {
messagesToAdd,
messagesToRemove,
removeAllRequested,
add,
remove,
removeAll,
addAlert
}
})
File: src/stores/queueStore.ts (lines 83-85)
get url(): string {
return api.apiURL('/view?' + this.urlParams)
}
🧠 Learnings used
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/composables/useUploadModelWizard.ts:207-219
Timestamp: 2026-01-06T19:17:12.355Z
Learning: In `src/platform/assets/composables/useUploadModelWizard.ts`, the `uploadModel()` function should have a concurrency guard at the beginning that checks if `isUploading.value` is already true and returns false early to prevent race conditions from multiple rapid invocations.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to src/**/*.{ts,tsx,js,jsx,vue} : Ask if there is a simpler way to introduce functionality; choose the simpler course when available
jtydhr88
force-pushed
the
thumbnail-3d-generation
branch
from
7dcc34f to
fd143c3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/extensions/core/load3d/Load3d.ts`:
- Around line 772-807: Replace direct calls to cameraManager.toggleCamera with
the class-level toggle that keeps controls and view helper in sync: call
this.toggleCamera('perspective') before capturing and
this.toggleCamera(savedCameraType) in the finally block instead of
this.cameraManager.toggleCamera(...). This ensures controlsManager.controls and
any view helper bindings are updated when switching cameras during the thumbnail
flow (references: Load3d.toggleCamera, cameraManager.toggleCamera,
controlsManager.controls).
In `@src/extensions/core/load3d/Load3dUtils.ts`:
- Around line 8-30: The generateThumbnailIfNeeded flow can throw unhandled
rejections because callers use void; wrap the entire body of
Load3dUtils.generateThumbnailIfNeeded in a try/catch that logs a single error on
failure (use processLogger or existing logger), preserving context (include
modelPath and folderType) and avoid rethrowing so fire-and-forget callers don't
produce unhandled promise rejections; keep the existing logic (splitFilePath,
getThumbnailFilename, fileExists, captureThumbnail, uploadThumbnail) inside the
try block and call logger.error once in the catch with the caught error details.
♻️ Duplicate comments (1)
src/platform/assets/components/Media3DTop.vue (1)
27-34: ResetthumbnailErrorwhen the asset changes.If the previous asset failed, the placeholder will persist for the next asset unless the error flag is reset. Based on learnings, prefer watch/watchEffect for this side effect.
🐛 Proposed fix
-import { computed, ref } from 'vue' +import { computed, ref, watchEffect } from 'vue' @@ const thumbnailError = ref(false) +watchEffect(() => { + if (asset?.src) { + thumbnailError.value = false + } +}) + const thumbnailSrc = computed(() => { if (!asset?.src) return '' return asset.src.replace(/([?&]filename=)([^&]*)/, '$1$2.png') })
Comment on lines
+772
to
+807
| if (savedCameraType !== 'perspective') { | ||
| this.cameraManager.toggleCamera('perspective') | ||
| } | ||
|
|
||
| const box = new THREE.Box3().setFromObject(this.modelManager.currentModel) | ||
| const size = box.getSize(new THREE.Vector3()) | ||
| const center = box.getCenter(new THREE.Vector3()) | ||
|
|
||
| const maxDim = Math.max(size.x, size.y, size.z) | ||
| const distance = maxDim * 1.5 | ||
|
|
||
| const cameraPosition = new THREE.Vector3( | ||
| center.x - distance * 0.8, | ||
| center.y + distance * 0.4, | ||
| center.z + distance * 0.3 | ||
| ) | ||
|
|
||
| this.cameraManager.perspectiveCamera.position.copy(cameraPosition) | ||
| this.cameraManager.perspectiveCamera.lookAt(center) | ||
| this.cameraManager.perspectiveCamera.updateProjectionMatrix() | ||
|
|
||
| if (this.controlsManager.controls) { | ||
| this.controlsManager.controls.target.copy(center) | ||
| this.controlsManager.controls.update() | ||
| } | ||
|
|
||
| const result = await this.sceneManager.captureScene(width, height) | ||
| return result.scene | ||
| } finally { | ||
| this.sceneManager.gridHelper.visible = savedGridVisible | ||
|
|
||
| if (savedCameraType !== 'perspective') { | ||
| this.cameraManager.toggleCamera(savedCameraType) | ||
| } | ||
| this.cameraManager.setCameraState(savedState) | ||
| this.controlsManager.controls?.update() |
Contributor
There was a problem hiding this comment.
Keep controls/view helper in sync when switching cameras.
cameraManager.toggleCamera(...) bypasses Load3d.toggleCamera, so controls and view helper may remain bound to the previous camera type after thumbnail capture. This can leave interactions broken if the user was in orthographic mode.
🐛 Proposed fix (minimal side effects)
- if (savedCameraType !== 'perspective') {
- this.cameraManager.toggleCamera('perspective')
- }
+ if (savedCameraType !== 'perspective') {
+ this.cameraManager.toggleCamera('perspective')
+ this.controlsManager.updateCamera(this.cameraManager.activeCamera)
+ this.viewHelperManager.recreateViewHelper()
+ }
@@
- if (savedCameraType !== 'perspective') {
- this.cameraManager.toggleCamera(savedCameraType)
- }
+ if (savedCameraType !== 'perspective') {
+ this.cameraManager.toggleCamera(savedCameraType)
+ this.controlsManager.updateCamera(this.cameraManager.activeCamera)
+ this.viewHelperManager.recreateViewHelper()
+ }🤖 Prompt for AI Agents
In `@src/extensions/core/load3d/Load3d.ts` around lines 772 - 807, Replace direct
calls to cameraManager.toggleCamera with the class-level toggle that keeps
controls and view helper in sync: call this.toggleCamera('perspective') before
capturing and this.toggleCamera(savedCameraType) in the finally block instead of
this.cameraManager.toggleCamera(...). This ensures controlsManager.controls and
any view helper bindings are updated when switching cameras during the thumbnail
flow (references: Load3d.toggleCamera, cameraManager.toggleCamera,
controlsManager.controls).
Comment on lines
+8
to
+30
| static async generateThumbnailIfNeeded( | ||
| load3d: Load3d, | ||
| modelPath: string, | ||
| folderType: 'input' | 'output' | ||
| ): Promise<void> { | ||
| const [subfolder, filename] = this.splitFilePath(modelPath) | ||
| const thumbnailFilename = this.getThumbnailFilename(filename) | ||
|
|
||
| const exists = await this.fileExists( | ||
| subfolder, | ||
| thumbnailFilename, | ||
| folderType | ||
| ) | ||
| if (exists) return | ||
|
|
||
| const imageData = await load3d.captureThumbnail(256, 256) | ||
| await this.uploadThumbnail( | ||
| imageData, | ||
| subfolder, | ||
| thumbnailFilename, | ||
| folderType | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Prevent unhandled rejections from fire‑and‑forget thumbnail generation.
Callers invoke this with void, so any failure will surface as an unhandled promise rejection. Wrap the flow in a try/catch and log once. As per coding guidelines, ensure proper error handling.
🐛 Proposed fix
static async generateThumbnailIfNeeded(
load3d: Load3d,
modelPath: string,
folderType: 'input' | 'output'
): Promise<void> {
- const [subfolder, filename] = this.splitFilePath(modelPath)
- const thumbnailFilename = this.getThumbnailFilename(filename)
-
- const exists = await this.fileExists(
- subfolder,
- thumbnailFilename,
- folderType
- )
- if (exists) return
-
- const imageData = await load3d.captureThumbnail(256, 256)
- await this.uploadThumbnail(
- imageData,
- subfolder,
- thumbnailFilename,
- folderType
- )
+ try {
+ const [subfolder, filename] = this.splitFilePath(modelPath)
+ const thumbnailFilename = this.getThumbnailFilename(filename)
+
+ const exists = await this.fileExists(
+ subfolder,
+ thumbnailFilename,
+ folderType
+ )
+ if (exists) return
+
+ const imageData = await load3d.captureThumbnail(256, 256)
+ await this.uploadThumbnail(
+ imageData,
+ subfolder,
+ thumbnailFilename,
+ folderType
+ )
+ } catch (error) {
+ console.warn('[Load3D] generateThumbnailIfNeeded failed:', error)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| static async generateThumbnailIfNeeded( | |
| load3d: Load3d, | |
| modelPath: string, | |
| folderType: 'input' | 'output' | |
| ): Promise<void> { | |
| const [subfolder, filename] = this.splitFilePath(modelPath) | |
| const thumbnailFilename = this.getThumbnailFilename(filename) | |
| const exists = await this.fileExists( | |
| subfolder, | |
| thumbnailFilename, | |
| folderType | |
| ) | |
| if (exists) return | |
| const imageData = await load3d.captureThumbnail(256, 256) | |
| await this.uploadThumbnail( | |
| imageData, | |
| subfolder, | |
| thumbnailFilename, | |
| folderType | |
| ) | |
| } | |
| static async generateThumbnailIfNeeded( | |
| load3d: Load3d, | |
| modelPath: string, | |
| folderType: 'input' | 'output' | |
| ): Promise<void> { | |
| try { | |
| const [subfolder, filename] = this.splitFilePath(modelPath) | |
| const thumbnailFilename = this.getThumbnailFilename(filename) | |
| const exists = await this.fileExists( | |
| subfolder, | |
| thumbnailFilename, | |
| folderType | |
| ) | |
| if (exists) return | |
| const imageData = await load3d.captureThumbnail(256, 256) | |
| await this.uploadThumbnail( | |
| imageData, | |
| subfolder, | |
| thumbnailFilename, | |
| folderType | |
| ) | |
| } catch (error) { | |
| console.warn('[Load3D] generateThumbnailIfNeeded failed:', error) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/extensions/core/load3d/Load3dUtils.ts` around lines 8 - 30, The
generateThumbnailIfNeeded flow can throw unhandled rejections because callers
use void; wrap the entire body of Load3dUtils.generateThumbnailIfNeeded in a
try/catch that logs a single error on failure (use processLogger or existing
logger), preserving context (include modelPath and folderType) and avoid
rethrowing so fire-and-forget callers don't produce unhandled promise
rejections; keep the existing logic (splitFilePath, getThumbnailFilename,
fileExists, captureThumbnail, uploadThumbnail) inside the try block and call
logger.error once in the catch with the caught error details.
christian-byrne
approved these changes
Jan 18, 2026
godwiniheuwa
pushed a commit
to godwiniheuwa/ComfyUI_frontend
that referenced
this pull request
Jan 22, 2026
## Summary add thrumbnail for 3d genations, feature requested by @PabloWiedemann ## Screenshots https://github.com/user-attachments/assets/4fb9b88b-dd7b-4a69-a70c-e850472d3498 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8129-add-thumbnail-for-3d-generation-2eb6d73d365081f2a30bc698a4fde6e0) by [Unito](https://www.unito.io)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
add thrumbnail for 3d genations, feature requested by @PabloWiedemann
Screenshots
2026-01-17.13-47-50.mp4
┆Issue is synchronized with this Notion page by Unito