fix(studio): make image background removal reliable - #3967
Closed
Miguel07Alm wants to merge 1 commit into
Closed
Miguel07Alm wants to merge 1 commit into
Miguel07Alm wants to merge 1 commit into
Conversation
Normalize project-local media src (including Studio preview URLs) so Remove BG works for images in both property panels, and cover the JPG-to-PNG default cutout path on the media route.
There was a problem hiding this comment.
🟡 Changes recommended
Moderate issues remain in path validation and nested-composition cutout path handling.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds shared project-asset path resolution for reliable background removal in Studio’s classic and flat media panels.
Changes:
- Normalize and validate project-local asset paths and preview URLs.
- Integrate resolution into both media panels.
- Add resolver, cutout-flow, and JPG-to-PNG server tests.
File summaries
| File | Summary | Review status |
|---|---|---|
packages/studio/src/utils/projectAssetPath.ts |
Shared asset-path resolver | Moderate changes needed for traversal rejection, origin/project validation, and nested bare-path handling. |
packages/studio/src/utils/projectAssetPath.test.ts |
Resolver tests | Add coverage for the identified traversal, external preview, and nested-source cases. |
packages/studio/src/components/editor/propertyPanelMediaSection.tsx |
Classic panel integration | Moderate change needed to write cutout paths relative to nested composition files. |
packages/studio/src/components/editor/propertyPanelMediaSection.test.tsx |
Classic panel cutout tests | Add nested-composition regression coverage. |
packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx |
Flat panel integration | Moderate change needed to write cutout paths relative to nested composition files. |
packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx |
Flat panel cutout tests | Add nested-composition regression coverage. |
packages/studio-server/src/routes/media.test.ts |
JPG-to-PNG route coverage | Adds coverage for the default PNG cutout output. |
Review details
Suppressed comments (3)
packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx:91
- Using
sourceFilehere makes the input path project-root relative for nested compositions, butapplyCutoutResultlater writes the server's project-root-relativeresult.outputPathdirectly back into the authoredsrc. For an image inscenes/intro.html,assets/cutouts/...pngresolves asscenes/assets/cutouts/...png, so the applied cutout is broken; convert the output back to a path relative toelement.sourceFileand cover the nested case.
const projectSrc = resolveProjectAssetPath(srcAttr, element.sourceFile || "index.html") ?? "";
packages/studio/src/components/editor/propertyPanelMediaSection.tsx:89
- Using
sourceFilehere makes the input path project-root relative for nested compositions, butapplyCutoutResultlater writes the server's project-root-relativeresult.outputPathdirectly back into the authoredsrc. For an image inscenes/intro.html,assets/cutouts/...pngresolves asscenes/assets/cutouts/...png, so the applied cutout is broken; convert the output back to a path relative toelement.sourceFileand cover the nested case.
const projectSrc = resolveProjectAssetPath(srcAttr, element.sourceFile || "index.html") ?? "";
packages/studio/src/utils/projectAssetPath.ts:91
- Only
./and../references should be resolved againstsourceFile; bare paths are the project-root convention in Studio previews. With this unconditional join,assets/photo.jpgfromcompositions/scene.htmlbecomescompositions/assets/photo.jpg, so a valid nested-composition asset is sent to the wrong server path. Keep bare paths root-relative and add a nested-source test.
if (!fromPreviewUrl) {
const sourceDir = sourceFile.includes("/")
? sourceFile.slice(0, sourceFile.lastIndexOf("/"))
: "";
path = sourceDir ? `${sourceDir}/${path}` : path;
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+31
to
+39
| function normalizeRelativeSegments(path: string): string { | ||
| const parts = path.split("/"); | ||
| const normalized: string[] = []; | ||
| for (const part of parts) { | ||
| if (!part || part === ".") continue; | ||
| if (part === "..") { | ||
| normalized.pop(); | ||
| continue; | ||
| } |
Comment on lines
+64
to
+68
| if (/^https?:\/\//i.test(path)) { | ||
| try { | ||
| path = new URL(path).pathname; | ||
| } catch { | ||
| return null; |
Collaborator
|
Superseded by #3969 — same change, opened from heygen-com as miguel-heygen (work). |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
resolveProjectAssetPathhelper so both classic and flat media property panels normalize project-local image/videosrcvalues (relative paths and Studio/api/projects/.../preview/...URLs) before calling remove-background.http(s),data:,blob:,file:, protocol-relative, and non-preview/apiURLs; strip query/hash; safely URI-decode; normalize.//..segments.Test plan
bun run --cwd packages/studio test src/utils/projectAssetPath.test.ts src/components/editor/propertyPanelFlatMediaSection.test.tsx src/components/editor/propertyPanelMediaSection.test.tsxbun run --cwd packages/studio-server test src/routes/media.test.tsbunx oxlint+bunx oxfmt --checkon changed filessrcis an/api/projects/.../preview/...URL), confirm Remove BG is enabled, and that the cutout applies as a PNG underassets/cutouts/