feat(spx-gui): support local image as reference for costume / animation generation#2902
feat(spx-gui): support local image as reference for costume / animation generation#2902cn0809 wants to merge 1 commit intogoplus:devfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the AI generation capabilities by allowing users to provide local images as references for creating costumes and animations. Previously, only existing costumes could be used as references. This change introduces greater flexibility and control for users, enabling them to leverage external visual assets directly within the generation workflow. The update involves a new UI component for reference image selection and corresponding backend model adjustments to manage and persist these new image references. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature allowing users to upload a local image as a reference for AI generation of sprites, costumes, and animations. The changes are well-structured, with a new reusable ReferenceImageInput.vue component that replaces the old ReferenceCostumeInput.vue. The data models (SpriteGen, CostumeGen, AnimationGen) are consistently updated to handle the new referenceImage property, including its serialization and deserialization. The overall implementation is solid. I've suggested a few improvements to enhance robustness during project loading by adding explicit checks for missing reference image files, which is more consistent with how other assets are handled in the codebase.
| if (referenceImagePath != null) { | ||
| const refFile = files[referenceImagePath] | ||
| if (refFile != null) inits.referenceImage = refFile | ||
| } |
There was a problem hiding this comment.
For better robustness and to ensure data integrity, it's good practice to throw an error if the referenceImagePath is present in the config but the corresponding file is not found in the project files. This makes debugging easier in case of corrupted project data and is consistent with how other file assets are loaded in the project.
| if (referenceImagePath != null) { | |
| const refFile = files[referenceImagePath] | |
| if (refFile != null) inits.referenceImage = refFile | |
| } | |
| if (referenceImagePath != null) { | |
| const refFile = files[referenceImagePath] | |
| if (refFile == null) throw new Error(`file ${referenceImagePath} not found for animation gen ${genId}`) | |
| inits.referenceImage = refFile | |
| } |
| if (referenceImagePath != null) { | ||
| const refFile = files[referenceImagePath] | ||
| if (refFile != null) inits.referenceImage = refFile | ||
| } |
There was a problem hiding this comment.
To improve robustness, we should throw an error if referenceImagePath is specified but the file is missing. This aligns with how other assets are loaded and helps detect data corruption issues early.
| if (referenceImagePath != null) { | |
| const refFile = files[referenceImagePath] | |
| if (refFile != null) inits.referenceImage = refFile | |
| } | |
| if (referenceImagePath != null) { | |
| const refFile = files[referenceImagePath] | |
| if (refFile == null) throw new Error(`file ${referenceImagePath} not found for costume gen ${genId}`) | |
| inits.referenceImage = refFile | |
| } |
| if (referenceImagePath != null) { | ||
| const refFile = files[referenceImagePath] | ||
| if (refFile != null) inits.referenceImage = refFile | ||
| } |
There was a problem hiding this comment.
It's better to throw an error when a referenceImagePath is defined but the file doesn't exist. This ensures data consistency and makes it easier to identify issues with corrupted project files, which is consistent with how other file loading is handled.
| if (referenceImagePath != null) { | |
| const refFile = files[referenceImagePath] | |
| if (refFile != null) inits.referenceImage = refFile | |
| } | |
| if (referenceImagePath != null) { | |
| const refFile = files[referenceImagePath] | |
| if (refFile == null) throw new Error(`file ${referenceImagePath} not found for sprite gen ${genId}`) | |
| inits.referenceImage = refFile | |
| } |
There was a problem hiding this comment.
Pull request overview
Adds support for using a locally uploaded image as the “reference” input for sprite/costume/animation generation, aligning with issue #2786 (including default costume generation).
Changes:
- Persist and load a
referenceImageforSpriteGen,CostumeGen, andAnimationGenviareferenceImagePathin serialized configs. - Use the uploaded reference image (when present) to produce
referenceImageUrl/referenceFrameUrlby uploading viasaveFile(). - Replace the old “reference costume” selector UI with a new
ReferenceImageInputthat supports either selecting an existing costume or uploading a local image.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| spx-gui/src/models/spx/gen/sprite-gen.ts | Adds referenceImage handling for default sprite image generation and persists it via referenceImagePath. |
| spx-gui/src/models/spx/gen/costume-gen.ts | Supports local referenceImage (mutually exclusive with referenceCostumeId) and persists it via referenceImagePath. |
| spx-gui/src/models/spx/gen/animation-gen.ts | Supports local referenceImage for video generation and persists it via referenceImagePath. |
| spx-gui/src/models/spx/gen/animation-gen.test.ts | Updates expected error message text to match new “reference image” requirement. |
| spx-gui/src/components/asset/gen/sprite/SpriteSettingsInput.vue | Adds reference image input to sprite generation settings UI. |
| spx-gui/src/components/asset/gen/costume/CostumeSettingsInput.vue | Switches from reference costume selector to the new reference image selector/uploader. |
| spx-gui/src/components/asset/gen/animation/AnimationSettingsInput.vue | Switches from reference costume selector to the new reference image selector/uploader. |
| spx-gui/src/components/asset/gen/common/ReferenceImageInput.vue | New component enabling “select costume OR upload local image” reference selection. |
| spx-gui/src/components/asset/gen/common/ReferenceCostumeInput.vue | Removed in favor of ReferenceImageInput. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const settings = this.getDefaultCostumeSettings() | ||
| if (this.referenceImage != null) { | ||
| settings.referenceImageUrl = await saveFile(this.referenceImage) | ||
| } |
There was a problem hiding this comment.
New behavior: SpriteGen.genImages() optionally uploads referenceImage and forwards it as settings.referenceImageUrl. There are existing comprehensive tests for SpriteGen, but none appear to cover this new branch (including export/load persistence via referenceImagePath). Add tests to verify the reference image is persisted and influences the task params when set.
| const refImg = this.referenceImage ?? this.referenceCostume?.img ?? null | ||
| const referenceImageUrl = refImg != null ? await saveFile(refImg) : null | ||
| const settings = { ...this.settings, referenceImageUrl } |
There was a problem hiding this comment.
CostumeGen.generate() now supports a local referenceImage (and falls back to referenceCostume.img). The CostumeGen test suite is extensive, but it doesn't currently cover this new precedence/mutual-exclusion behavior or referenceImagePath export/load. Add tests that set referenceImage, verify saveFile is called with it (instead of the costume), and that the reference image survives export/load.
| const refImg = this.referenceImage ?? this.referenceCostume?.img ?? null | ||
| if (refImg == null) throw new Error('reference image expected') | ||
| const referenceFrameUrl = await saveFile(refImg) |
There was a problem hiding this comment.
AnimationGen.generateVideo() now allows referenceImage (local upload) in addition to referenceCostume.img. The existing AnimationGen tests cover many flows but don't validate this new path or the new referenceImagePath export/load behavior. Add coverage to ensure a provided referenceImage is used to produce referenceFrameUrl, and that it round-trips through export/load.
| if (selectedItem != null) inits.selectedItem = selectedItem | ||
| if (referenceImagePath != null) { | ||
| const refFile = files[referenceImagePath] | ||
| if (refFile != null) inits.referenceImage = refFile |
There was a problem hiding this comment.
When referenceImagePath is present in the saved config but the file is missing from files, the loader silently ignores it and proceeds with referenceImage = null. This can hide corrupted/partial project data and lead to unexpected behavior later. Consider throwing an error (consistent with how missing imagePath / generated file paths are handled) or explicitly clearing referenceImagePath during export if optional.
| if (refFile != null) inits.referenceImage = refFile | |
| if (refFile == null) { | |
| throw new Error(`reference image file ${referenceImagePath} not found for sprite gen ${genId}`) | |
| } | |
| inits.referenceImage = refFile |
| if (referenceCostumeId != null) inits.referenceCostumeId = referenceCostumeId | ||
| if (referenceImagePath != null) { | ||
| const refFile = files[referenceImagePath] | ||
| if (refFile != null) inits.referenceImage = refFile |
There was a problem hiding this comment.
referenceImagePath is handled differently from other persisted file paths: if the config contains it but the corresponding entry is missing from files, it is silently ignored. This can cause silent loss of the user's chosen reference image on reload. Consider throwing when referenceImagePath is set but files[referenceImagePath] is missing (similar to imagePath / generated paths).
| if (refFile != null) inits.referenceImage = refFile | |
| if (refFile == null) throw new Error(`file ${referenceImagePath} not found for costume gen ${genId}`) | |
| inits.referenceImage = refFile |
| if (referenceCostumeId != null) inits.referenceCostumeId = referenceCostumeId | ||
| if (referenceImagePath != null) { | ||
| const refFile = files[referenceImagePath] | ||
| if (refFile != null) inits.referenceImage = refFile |
There was a problem hiding this comment.
If referenceImagePath is present in the serialized config but the file entry is missing from files, the loader currently just skips setting referenceImage. That silently drops user data and is inconsistent with the strict handling for videoPath / generated paths. Consider throwing an error (or otherwise surfacing the corrupted state) when referenceImagePath is set but not found.
| if (refFile != null) inits.referenceImage = refFile | |
| if (refFile == null) throw new Error(`file ${referenceImagePath} not found for animation gen ${genId}`) | |
| inits.referenceImage = refFile |
![xgopilot[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
|
|
||
| <style lang="scss" scoped> | ||
| .param-button { | ||
| height: 32px; |
There was a problem hiding this comment.
uploadedFile.value can be null here. uploadedFile is only updated to a non-null value when props.referenceImage arrives as non-null (line 154), so if the component mounts with referenceImage: null, uploadedFile stays null and this emit fires update:referenceImage with null, which then clears selectedCostumeId for no reason.
The v-if="uploadedFile != null" guard on the enclosing UIBlockItem prevents this in normal flow, but a defensive null guard in the handler body would make the invariant explicit:
| height: 32px; | |
| emit('update:referenceImage', uploadedFile.value) |
→
if (uploadedFile.value == null) return
emit('update:referenceImage', uploadedFile.value)
emit('update:selectedCostumeId', null)
| const iconOnly = computed(() => settingsInputCtx.iconOnly) | ||
|
|
||
| // Costume options with resolved image URLs | ||
| const costumeOptions = useAsyncComputed((onCleanup) => { |
There was a problem hiding this comment.
useAsyncComputed re-runs the entire computation (calling c.img.url(onCleanup) for every costume) whenever props.costumes reactively changes. Each run resets costumeOptions.value to null first, causing a brief visual flash of the dropdown losing all costume options. With N costumes, adding a single new costume triggers N blob-URL recreations.
This mirrors a pre-existing pattern in the codebase, so no immediate fix is required, but it's worth noting as a known UX/perf tradeoff that grows with the number of costumes in the sprite.
| } | ||
|
|
||
| if (this.referenceImage != null) { | ||
| const refPath = `${assetsPath}/referenceImage${extname(this.referenceImage.name)}` |
There was a problem hiding this comment.
extname(this.referenceImage.name) extracts the extension verbatim from the user-supplied browser File.name. While the path is only written into the in-memory virtual filesystem (and later into a ZIP archive), two concerns arise:
-
Non-image extensions:
imgExtsincludessvg. An SVG with embedded<script>content will be accepted by the browser file picker, uploaded verbatim to cloud storage, and passed asreferenceImageUrlto the AI API. There is no server-side MIME or content validation. If the cloud-storage URL is ever navigated to directly or embedded as<object>/<embed>, the script executes. -
Extension injection into ZIP paths: If a filename contains an unusual extension (e.g.
.html,.php), it is written directly into the ZIP archive key. Thexbp.tssave path uses allfileskeys verbatim as ZIP entry names with no sanitization.
Consider whitelisting the extension against imgExts before constructing the path (e.g. fall back to .bin for unknown extensions), and document that SVG uploads may need server-side sanitization before rendering.
| if (referenceCostumeId != null) inits.referenceCostumeId = referenceCostumeId | ||
| if (referenceImagePath != null) { | ||
| const refFile = files[referenceImagePath] | ||
| if (refFile != null) inits.referenceImage = refFile |
There was a problem hiding this comment.
When referenceImagePath is recorded in the serialized config but the corresponding file is absent from files, the reference image is silently dropped. A reloaded session would then have no reference set, restoring to a different state than what was serialized — silent data corruption.
Additionally: referenceImagePath is read from a potentially untrusted JSON config (e.g., from a shared .xbp file). A crafted config could set this path to any key in files (e.g. a project source file), causing it to be treated as an image reference and uploaded to the AI API. Consider:
- Throwing (or at minimum warning) when
referenceImagePathis set but the file is missing - Verifying the resolved file's MIME type before setting it as
referenceImage
| } | ||
| if (imagePath != null) config.imagePath = imagePath | ||
| if (this.referenceImage != null) { | ||
| const refPath = `${assetsPath}/referenceImage${extname(this.referenceImage.name)}` |
There was a problem hiding this comment.
Same as animation-gen.ts: extname(this.referenceImage.name) uses the user-supplied filename verbatim, and referenceImagePath on load is accepted from persisted config without MIME-type validation. See the comment on animation-gen.ts:309 and :348.
| this.referenceCostumeId = costumeId | ||
| if (costumeId != null) this.referenceImage = null | ||
| } | ||
|
|
There was a problem hiding this comment.
The setReferenceImage method silently clears referenceCostumeId (and vice versa), but this mutual-exclusivity invariant is undocumented on the public API. Callers reading referenceImage or referenceCostumeId in isolation won't know that setting one nulls the other. A short JSDoc comment on both setters would prevent future bugs:
/**
* Sets a local image file as the generation reference.
* Mutually exclusive with referenceCostumeId — clears any selected costume reference.
*/
setReferenceImage(file: File | null) { ... }
|
Good feature addition — the A few issues worth addressing:
See inline comments for specifics. |
close: #2786