feat(studio): 4-stage inpaint pipeline with compose view - #8
Merged
Conversation
Pipeline (C#):
- Save white-fill inpainted.png per job (WhiteFillAll); architecture
cleanly separates inpainting (stage 2) from text rendering (stage 4)
so a real LaMa/diffusion model can drop in without structural changes
- Add RenderTextOnly() — draws text glyphs with no white-fill so the
bubble background comes entirely from the pre-inpainted image
- TranslatePageAsync step 4 now uses RenderTextOnly(inpaintedPng)
instead of RenderTranslations(originalPng), eliminating white-box
double-fill artifacts in result.png
- RerenderAsync reads inpainted.png as base if present; falls back to
original.png + RenderTranslations for pre-existing jobs
- Add InpaintedImagePath column to PageTranslationJob (EF migration)
- Add GET /api/portal/jobs/{id}/inpainted route
Studio UI (SolidJS):
- Replace ViewMode with Stage: original | inpainted | compose | result
- Toolbar shows 4 stage buttons (1·Original … 4·Result); click to
toggle, max 2 active simultaneously → side-by-side panels
- Compose stage (3) shows inpainted.png as background with translated
text overlaid live via SVG foreignObject (transparent bg, no burn)
- Add showTextOverlay prop to BubbleCanvas; renders translatedText
centered inside each non-excluded bubble with no white fill
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe translation pipeline now stores an inpainted image separately from the final result. A new portal endpoint and client URL expose it, while Studio supports selecting up to two pipeline stages and BubbleCanvas can display translated text overlays. ChangesInpainted rendering pipeline
Studio stage viewer
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant StudioPage
participant PortalRoutes
participant PageTranslationService
participant TypesettingService
participant PageTranslationJob
PageTranslationService->>TypesettingService: WhiteFillAll(originalPng, bubbleRegions)
TypesettingService-->>PageTranslationService: inpaintedPng
PageTranslationService->>TypesettingService: RenderTextOnly(inpaintedPng, translations)
TypesettingService-->>PageTranslationService: resultPng
PageTranslationService->>PageTranslationJob: Persist image paths
StudioPage->>PortalRoutes: Request selected inpainted stage
PortalRoutes-->>StudioPage: Return inpainted PNG
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/ClientApp/src/pages/StudioPage.tsx (1)
257-264: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRefetch the job after rerender, redetect, and retranslate complete.
The page polls for completion with standalone
getJobcalls, but the reactivejob()resource used by the stage renderers is not updated. This leaves the UI showing stale placeholders until another resource refresh or page reload.Proposed fix
- const [job] = createResource(() => params.id, getJob); + const [job, { refetch: refetchJob }] = createResource(() => params.id, getJob); ... await pollUntilDone(); await refetchBubbles(); + await refetchJob(); setImageVersion((v) => v + 1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/ClientApp/src/pages/StudioPage.tsx` around lines 257 - 264, Update handleRerender so the reactive job() resource is explicitly refetched after rerenderJob, pollUntilDone, and refetchBubbles complete. Reuse the existing job resource’s refetch mechanism, ensuring stage renderers receive the latest job state immediately while preserving the current image-version update flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/ClientApp/src/components/BubbleCanvas.tsx`:
- Around line 534-550: Update the preview style in BubbleCanvas’s bubble
renderer to use the per-bubble fontFamily value and derive font-size from
fontSizeOverride multiplied by layout().scale when an override exists, while
retaining the current height-based fallback otherwise.
In `@server/ClientApp/src/pages/StudioPage.tsx`:
- Around line 421-444: Update the stage toggle buttons rendered in the
ALL_STAGES loop to include aria-pressed bound to isActive(), exposing each
button’s selected state to assistive technology while preserving the existing
toggle behavior.
In `@server/src/Services/PageTranslationService.cs`:
- Around line 263-272: The rerender path must rebuild the inpainted base from
original.png before rendering current bubble data. In the flow containing
basePath and RenderTextOnly, regenerate and persist inpainted.png under the
existing lock, then read that freshly generated image and pass it to
RenderTextOnly; preserve the original-image fallback and RenderTranslations
behavior for jobs without an inpainted image.
- Around line 152-155: Update the inpainting call in the PageTranslationService
flow to pass an input derived from all detected bubbles in bubbles, rather than
the translations collection that excludes failed or empty OCR results. Preserve
the existing WhiteFillAll, file-writing, and path-generation behavior.
---
Outside diff comments:
In `@server/ClientApp/src/pages/StudioPage.tsx`:
- Around line 257-264: Update handleRerender so the reactive job() resource is
explicitly refetched after rerenderJob, pollUntilDone, and refetchBubbles
complete. Reuse the existing job resource’s refetch mechanism, ensuring stage
renderers receive the latest job state immediately while preserving the current
image-version update flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: be38da88-271a-42ec-b2d4-e173e446a60c
⛔ Files ignored due to path filters (2)
server/wwwroot/js/app.cssis excluded by!server/wwwroot/js/app.cssserver/wwwroot/js/app.jsis excluded by!server/wwwroot/js/app.js
📒 Files selected for processing (11)
server/ClientApp/src/api.tsserver/ClientApp/src/components/BubbleCanvas.tsxserver/ClientApp/src/pages/StudioPage.tsxserver/ClientApp/src/types.tsserver/Migrations/20260730105929_AddInpaintedImagePath.Designer.csserver/Migrations/20260730105929_AddInpaintedImagePath.csserver/Migrations/AppDbContextModelSnapshot.csserver/src/Data/AppDbContext.csserver/src/Routes/PortalRoutes.csserver/src/Services/PageTranslationService.csserver/src/Services/TypesettingService.cs
- PageTranslationService: pass all detected bubbles (not just those with translations) to WhiteFillAll so OCR-failed bubbles still get their background cleaned - RerenderAsync: regenerate inpainted.png from current bubble positions before rendering text, so moved/resized boxes are reflected in the clean background layer; persist updated InpaintedImagePath to DB - handleRerender: refetch job resource alongside bubbles after re-render so stage renderers see the latest inpaintedImagePath/resultImagePath - BubbleCanvas compose overlay: respect bubble.fontFamily and derive font-size from fontSizeOverride * layout().scale when override is set - Stage picker buttons: add aria-pressed bound to isActive() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/src/Services/PageTranslationService.cs`:
- Around line 264-272: Update TranslatePageAsync so every detected bubble
persists an empty translation log, including OCR failure and empty-text early
exits, before returning. Reuse LogBubbleAsync for these paths and ensure the
stored BubbleX, BubbleY, BubbleW, BubbleH, and Confidence values allow the
allBubbleLogs/allBubbleFills flow in PageTranslationService to include the
geometry during rerender.
- Around line 294-301: Update the regeneration flow around allBubbleFills and
the basePng/resultPng selection so an existing inpaintedPath is used only when
current bubble fills were generated. When allBubbleFills.Count is zero, clear
InpaintedImagePath and delete the stale inpainted file, then use originalPng and
the legacy RenderTranslations path; preserve the current inpainted rendering
path when valid fills exist.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e6ba929c-db60-42d5-bdba-d87b4efa3773
📒 Files selected for processing (3)
server/ClientApp/src/components/BubbleCanvas.tsxserver/ClientApp/src/pages/StudioPage.tsxserver/src/Services/PageTranslationService.cs
Comment on lines
+294
to
+301
| // Render text on the freshly regenerated inpainted base, or fall back | ||
| // to the legacy white-fill+text path for jobs without bubble data. | ||
| var basePng = File.Exists(inpaintedPath) | ||
| ? await File.ReadAllBytesAsync(inpaintedPath, ct) | ||
| : originalPng; | ||
| var resultPng = File.Exists(inpaintedPath) | ||
| ? await Task.Run(() => typesetter.RenderTextOnly(basePng, translations, padding), ct) | ||
| : await Task.Run(() => typesetter.RenderTranslations(basePng, translations, padding), ct); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not reuse a stale inpainted file when no current bubble fills exist.
When all current bubbles are excluded or absent, regeneration is skipped, but File.Exists(inpaintedPath) still selects an older inpainted.png. Rerender then produces a stale white-filled result instead of falling back to original.png, while the Studio inpainted stage may remain advertised. Gate base selection on current inpaint data and clear the persisted inpainted path when none exists.
Proposed fix
+ var hasCurrentInpainted =
+ allBubbleFills.Count > 0 && File.Exists(inpaintedPath);
var basePng = File.Exists(inpaintedPath)
- ? await File.ReadAllBytesAsync(inpaintedPath, ct)
+ ? await File.ReadAllBytesAsync(inpaintedPath, ct)
: originalPng;
- var resultPng = File.Exists(inpaintedPath)
+ var resultPng = hasCurrentInpainted
? await Task.Run(() => typesetter.RenderTextOnly(basePng, translations, padding), ct)
: await Task.Run(() => typesetter.RenderTranslations(basePng, translations, padding), ct);Also clear InpaintedImagePath and remove the stale file when allBubbleFills.Count == 0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/src/Services/PageTranslationService.cs` around lines 294 - 301, Update
the regeneration flow around allBubbleFills and the basePng/resultPng selection
so an existing inpaintedPath is used only when current bubble fills were
generated. When allBubbleFills.Count is zero, clear InpaintedImagePath and
delete the stale inpainted file, then use originalPng and the legacy
RenderTranslations path; preserve the current inpainted rendering path when
valid fills exist.
- Log bubble geometry for OCR-failure and empty-text paths so RerenderAsync can still inpaint those bubbles from the DB. - Clear stale inpainted.png (file + DB column) in RerenderAsync when allBubbleFills is empty, preventing RenderTextOnly from running on a stale inpainted base. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
This was referenced Jul 31, 2026
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
1 participant
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
WhiteFillAllmethod cleanly separates the inpainting stage (stage 2) from text rendering (stage 4). A real LaMa/diffusion model can replace the white-fill placeholder without structural changes to the pipeline.RenderTextOnly()draws text glyphs with no white-fill;TranslatePageAsyncstep 4 now calls it withinpaintedPngas the base instead ofRenderTranslationson the original. Eliminates the white-box double-fill artifact inresult.png.RerenderAsyncbackward-compatible — readsinpainted.pngas base if it exists, falls back tooriginal.png + RenderTranslationsfor jobs created before this change.InpaintedImagePathcolumn toPageTranslationJob; EF migration included.GET /api/portal/jobs/{id}/inpaintedserves the inpainted image.ViewModetoggle with a stage picker (1·Original,2·Inpainted,3·Compose,4·Result). Click to toggle; max 2 active at once → side-by-side panels. Clicking a third stage replaces the oldest.inpainted.pngas the background with translated text overlaid live via SVG<foreignObject>(transparent background, no burn) so the user can review/adjust layout before committing to Re-render.showTextOverlayprop onBubbleCanvas— renderstranslatedTextcentered inside each non-excluded bubble with no white fill behind the glyphs.Test plan
inpainted.pngis saved alongsideresult.pngresult.pnghas text rendered on clean white-filled bubbles (no double white-box)RerenderAsyncusesinpainted.pngas baseinpainted.png→ confirm fallback to original + white-fill still worksGET /api/portal/jobs/{id}/inpaintedreturns 200 with valid PNG🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements