feat(inpaint): TextSeg-guided two-pass inpainting with pixel-accurate erasure - #11
Merged
Conversation
… erasure Introduces a full text-erasure pipeline powered by ComicTextDetector (TextSeg): **TextSeg service** - Add TextSegmentationService running comictextdetector ONNX model - Outputs per-pixel sigmoid mask (text on white) + bounding boxes for all text blocks - TextSeg runs first so its results feed both OCR and inpainting **Two-pass inpainting** - Pass 0 — Telea (OpenCvSharp4): erases non-bubble TextSeg blocks (narration boxes, SFX, panel labels) using bounding-box mask; surrounding dark/light context is naturally reconstructed, no white rectangles - Pass 1 — LaMa ONNX with TextSeg pixel mask: erases text inside speech bubbles per-bubble using the precise pixel mask, not the bounding box **MaskComposite fix** - InpaintPageWithPixelMask previously used CopyRegion (whole bounding box) which overwrote bubble outlines and artwork with LaMa's reconstruction - Replaced with MaskComposite: only writes pixels where rawMask is true, preserving every non-text pixel (bubble borders, panel art, backgrounds) **Studio improvements** - BubbleCanvas showBubbles prop now correctly forwarded to the Inpainted stage so the bubble visibility toggle works in all Studio views - TextSeg block overlay for OCR/inpaint region visualisation - Bubble box management UI in Studio 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 (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe extension version changes to 1.3.7. The server adds TextSeg model management, ONNX segmentation, text-block APIs, canvas overlays, mask caching, and mask-based translation and inpainting. ChangesText segmentation runtime
Text block management
Translation and inpainting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PageTranslationService
participant InferenceWorker
participant TextSegmentationService
participant InpaintService
participant TypesettingService
PageTranslationService->>InferenceWorker: Queue TextSegJob
InferenceWorker->>TextSegmentationService: SegmentText(imagePng)
TextSegmentationService-->>InferenceWorker: Return mask and text blocks
InferenceWorker-->>PageTranslationService: Return TextSegResult
PageTranslationService->>InpaintService: InpaintPageWithPixelMask
PageTranslationService->>TypesettingService: TeleaInpaintBlocks
InpaintService-->>PageTranslationService: Return processed image
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/Services/InpaintService.cs (1)
400-411: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the stale summary block left above
MaskComposite.The diff inserts a new
<summary>forMaskCompositeat lines 404-408 but leaves the previous<summary>at lines 400-403 in place. That older block documentsCopyRegion. Two consecutive<summary>elements now attach toMaskComposite, which produces a duplicate-tag documentation warning and describes the wrong behavior.CopyRegionat line 446 is left with no documentation.♻️ Proposed fix
- /// <summary> - /// Copy a rectangular region from <paramref name="src"/> into <paramref name="dst"/> - /// at the specified destination offset. Both bitmaps must be BGRA8888. - /// </summary> /// <summary> /// Copy pixels from <paramref name="src"/> to <paramref name="dst"/> only where /// <paramref name="mask"/> (model-space boolean array, size <paramref name="maskSize"/>²) /// is <c>true</c>. Coordinates map via <paramref name="scale"/> (crop→model). /// </summary>Then restore the original summary above
CopyRegion:/// <summary> /// Copy a rectangular region from <paramref name="src"/> into <paramref name="dst"/> /// at the specified destination offset. Both bitmaps must be BGRA8888. /// </summary> private static unsafe void CopyRegion(🤖 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/InpaintService.cs` around lines 400 - 411, Remove the stale CopyRegion summary immediately before MaskComposite, leaving only MaskComposite’s mask-compositing documentation there. Restore that rectangular-copy summary directly above the CopyRegion method declaration so each method has exactly its own documentation.
🧹 Nitpick comments (7)
server/src/Services/TextSegmentationService.cs (4)
46-58: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueDispose a previous session and publish
_sessionlast.
LoadModelruns once fromBootBackgroundServicetoday, so this is defensive only. If it is ever called twice, the earlierInferenceSessionand its native arena leak. Assigning_sessionbefore_inputNamealso makesIsReadyreport true for a moment while_inputNameis still null.♻️ Proposed refactor
- _session = new InferenceSession(modelPath, opts); - _inputName = _session.InputNames[0]; // typically "images" for YOLO-style models + var session = new InferenceSession(modelPath, opts); + _inputName = session.InputNames[0]; // typically "images" for YOLO-style models + _session?.Dispose(); + _session = session;🤖 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/TextSegmentationService.cs` around lines 46 - 58, Update TextSegmentationService.LoadModel to create and fully initialize a new InferenceSession and input name in local variables before publishing them to the service fields. Dispose the existing _session before replacing it, and assign _inputName before _session so IsReady cannot observe a partially initialized model.
115-122: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDerive separate width and height scales for the crop offsets.
mScaleis computed frommWonly and then applied topadTopandnewHas well. The offsets are correct while the model output is square. They shift vertically ifmHever differs frommW. The ternary at Line 118 also returns the same value in both branches.♻️ Proposed refactor
- double mScale = mW == ModelInput ? 1.0 : (double)mW / ModelInput; - int cLeft = (int)Math.Round(padLeft * mScale); - int cTop = (int)Math.Round(padTop * mScale); - int cW = Math.Max(1, Math.Min(mW - cLeft, (int)Math.Round(newW * mScale))); - int cH = Math.Max(1, Math.Min(mH - cTop, (int)Math.Round(newH * mScale))); + double sx = (double)mW / ModelInput; + double sy = (double)mH / ModelInput; + int cLeft = (int)Math.Round(padLeft * sx); + int cTop = (int)Math.Round(padTop * sy); + int cW = Math.Max(1, Math.Min(mW - cLeft, (int)Math.Round(newW * sx))); + int cH = Math.Max(1, Math.Min(mH - cTop, (int)Math.Round(newH * sy)));🤖 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/TextSegmentationService.cs` around lines 115 - 122, In the crop calculation, replace the single mScale used by the crop block with independent width and height scales derived from mW and mH relative to ModelInput. Apply the width scale to padLeft and newW, and the height scale to padTop and newH, preserving the existing rounding and clamping behavior in cLeft, cTop, cW, and cH.
94-110: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSelect the mask output by name, not by index.
When neither candidate has one channel, the fallback selects
out1, which can belines_mapand produce an incorrect mask. Resolve the output by name, then use the channel count as a fallback. Theresults[1]andresults[2]indexers are supported by ONNX Runtime 1.26.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/TextSegmentationService.cs` around lines 94 - 110, Update the mask-selection logic in the segmentation output handling to resolve the mask tensor by its ONNX output name first, rather than assuming a positional index. Retain channel-count detection as the fallback when names do not identify the mask, and remove the unsafe fallback that can select lines_map; continue throwing when no usable mask output is available.
191-214: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse the existing dense output buffer.
AsTensor<float>()returns aDenseTensor<float>for numeric outputs in ONNX Runtime 1.26.0. Cast it, check!IsReversedStride, and readBuffer.Span[y * w + x]. Do not callToDenseTensor(), because it copies the complete output.🤖 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/TextSegmentationService.cs` around lines 191 - 214, Update BuildBinaryMask to accept the DenseTensor<float> output from AsTensor<float>, validate that IsReversedStride is false, and read values from Buffer.Span using the y * w + x offset instead of multidimensional indexing. Avoid ToDenseTensor and preserve the existing thresholding and bitmap-writing behavior.server/src/Routes/PortalRoutes.cs (2)
173-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the
:introute constraint and return the JSON directly.Sibling routes in this file use
{bubbleIndex:int}. Use the same constraint here so non-numeric segments do not reach the handler. Also, the re-serialization step is redundant: the elements are already parsed JSON, soPropertyNamingPolicyhas no effect on them, andDeserialize<object>only round-trips the string again.♻️ Proposed simplification
- g.MapDelete("/jobs/{id}/textseg-blocks/{index}", async ( + g.MapDelete("/jobs/{id}/textseg-blocks/{index:int}", async ( string id, int index, AppDbContext db, PageTranslationService pipeline) => @@ blocks.RemoveAt(index); - var updated = System.Text.Json.JsonSerializer.Serialize(blocks, - new System.Text.Json.JsonSerializerOptions - { - PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.SnakeCaseLower, - }); + var updated = System.Text.Json.JsonSerializer.Serialize(blocks); await File.WriteAllTextAsync(cacheFile, updated); - return Results.Ok(System.Text.Json.JsonSerializer.Deserialize<object>(updated)); + return Results.Content(updated, "application/json");Also applies to: 191-197
🤖 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/Routes/PortalRoutes.cs` around lines 173 - 176, Update the MapDelete route for jobs/{id}/textseg-blocks/{index} and the corresponding sibling route to constrain the parameter as {index:int}, matching the existing bubbleIndex route pattern. Return the parsed JSON elements directly from the handler and remove the redundant Deserialize<object>/re-serialization step and its unused naming-policy handling.
137-141: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConfirm the cached JSON stays in sync with block deletions and re-runs.
The fast path returns
textseg_blocks.jsonverbatim.PageTranslationService.TranslatePageAsyncalso writes this file, and the DELETE endpoint rewrites it. Two parallel GET requests on a cold cache can both run segmentation and then write the same file concurrently, which can fail with anIOExceptionor produce a partially written file. Consider serializing the write with the existing per-job lock, or write to a temp file and move it atomically.♻️ Proposed atomic write
- await File.WriteAllTextAsync(cacheFile, json); + var tmp = cacheFile + ".tmp"; + await File.WriteAllTextAsync(tmp, json); + File.Move(tmp, cacheFile, overwrite: true); return Results.Content(json, "application/json");🤖 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/Routes/PortalRoutes.cs` around lines 137 - 141, Make cached JSON writes in the page translation and block-deletion flows atomic and safe under concurrent GET requests. Reuse the existing per-job lock when writing textseg_blocks.json, or write the complete content to a temporary file and atomically replace the cache file; ensure PortalRoutes’ cache read never observes partial JSON or fails due to parallel writes.server/src/Services/PageTranslationService.cs (1)
767-778: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the centre-containment test and accept a
CancellationToken.
GetTypesettingBoxat lines 769-776 andIsInsideAnyBubbleat lines 522-526 implement the same centre-in-box test. Extract one helper so the two stay consistent.
LoadTextSegMaskAsyncdoes not accept aCancellationToken, unlike the other async methods in this class. Both call sites, at line 349 and line 424, already hold act.♻️ Proposed refactor
+ /// <summary>Returns true when the centre of <paramref name="region"/> lies inside <paramref name="box"/>.</summary> + private static bool CenterInside(BubbleBox region, BubbleBox box) + { + float cx = region.X + region.Width / 2f; + float cy = region.Y + region.Height / 2f; + return cx >= box.X && cx <= box.X + box.Width && + cy >= box.Y && cy <= box.Y + box.Height; + } + private static BubbleBox GetTypesettingBox(BubbleBox textRegion, IReadOnlyList<BubbleBox> bubbles) { - float cx = textRegion.X + textRegion.Width / 2f; - float cy = textRegion.Y + textRegion.Height / 2f; foreach (var bubble in bubbles) - { - if (cx >= bubble.X && cx <= bubble.X + bubble.Width && - cy >= bubble.Y && cy <= bubble.Y + bubble.Height) - return bubble; - } + if (CenterInside(textRegion, bubble)) return bubble; return textRegion; } @@ - private async Task<byte[]?> LoadTextSegMaskAsync(string jobId) + private async Task<byte[]?> LoadTextSegMaskAsync(string jobId, CancellationToken ct = default) { var maskPath = Path.Combine(config.JobsDir, jobId, "textseg_mask.png"); - return File.Exists(maskPath) ? await File.ReadAllBytesAsync(maskPath) : null; + return File.Exists(maskPath) ? await File.ReadAllBytesAsync(maskPath, ct) : null; }Then simplify
IsInsideAnyBubble:private static bool IsInsideAnyBubble(BubbleBox block, IReadOnlyList<BubbleBox> bubbles) => bubbles.Any(b => CenterInside(block, b));Also applies to: 784-788
🤖 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 767 - 778, Extract the shared centre-containment logic from GetTypesettingBox and IsInsideAnyBubble into one helper, such as CenterInside, and reuse it in both methods. Update LoadTextSegMaskAsync to accept a CancellationToken and propagate the existing ct from both call sites through its async operations.
🤖 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/pages/StudioPage.tsx`:
- Around line 132-140: Update the TextSeg loading catch block surrounding
getJobTextSegBlocks to capture the failure and report it through setActionError,
matching the existing error-handling path used by handleDeleteTextSeg, while
preserving setShowTextSeg(false) and the finally cleanup.
- Line 890: Update the delete control’s visibility classes in the StudioPage JSX
so keyboard focus also reveals it, adding a focus-visible-based visibility rule
alongside group-hover:visible while preserving the existing styling and
pointer-hover behavior.
- Around line 874-896: Restructure the TextSeg row around the existing selection
handler so the selectable area is a button, making it keyboard-accessible and
announced as interactive. Move the delete button currently inside the row to be
a sibling outside the selectable button, while preserving event propagation
prevention, styling, and selection behavior without nesting buttons.
In `@server/server-csharp.csproj`:
- Around line 36-37: Add the OpenCvSharp native runtime package references for
the supported Windows and macOS platforms alongside OpenCvSharp4 and the
existing Linux x64 runtime in the project configuration. Use the appropriate
Windows and macOS runtime package variants while preserving the current Linux
runtime reference.
In `@server/src/BootExtensions.cs`:
- Around line 213-229: Update DownloadTextSegAsync to catch failures from
ModelDownloader.EnsureAsync, log a warning with the download context and
exception, and return without rethrowing so the optional TextSeg model failure
does not fault ExecuteAsync or the hosted service. Follow the existing non-fatal
handling pattern used by DownloadDictionaryAsync.
In `@server/src/Routes/PortalRoutes.cs`:
- Around line 172-198: Update the pipeline flow used by TranslatePageAsync and
its OCR/rendering stages so it reads the edited textseg_blocks.json cache and
preserves deleted blocks across subsequent runs; alternatively remove the
MapDelete endpoint if deletion is intentionally display-only. Ensure the
endpoint’s persisted changes are consumed by the actual processing path rather
than only returned in the overlay response.
In `@server/src/Services/PageTranslationService.cs`:
- Around line 196-210: Update the OCR processing flow around GetTypesettingBox,
translations.Add, and LogBubbleAsync to group regions by their typesetting
target before translating. Use value-based BubbleBox grouping, or group by its
X/Y/Width/Height tuple when necessary, join each group’s source text, and
perform one OCR/translation pass using the group’s union rectangle. Emit one
BubbleTranslation and one log row per group so downstream rendering and
rerendering process each bubble only once.
- Around line 349-352: Add a shared cached TextSeg loader in
PageTranslationService that reads textseg_mask.png and deserializes
textseg_blocks.json into BubbleBox values, warning and falling back to an empty
block list if parsing fails. Update RerenderAsync at
server/src/Services/PageTranslationService.cs:349-352 and InpaintOnlyAsync at
server/src/Services/PageTranslationService.cs:424-428 to use this loader and
pass the blocks into TextSegResult, verifying the constructor and BubbleBox
argument order.
- Around line 485-501: Guard the pixel-accurate path in TranslatePageAsync so a
preSeg.Mask is reused only when it has a nonzero length. Treat an empty mask
like an unavailable segmentation result and fall back to the existing
rectangle-mask path instead of passing it to InpaintWithMaskJob; preserve the
current behavior for valid masks and TextSegService-generated masks.
- Around line 79-90: Update the bubble-detection flow around
PageTranslationService’s bubbleTask so BubbleDetectionService.Detect is
submitted through InferenceQueue and processed by InferenceWorker, rather than
invoked via Task.Run; ensure the same queued handling is used by the re-render
path. Preserve result propagation and cancellation while maintaining
single-reader serialization with TextSegJob.
In `@server/src/Services/TextSegmentationService.cs`:
- Around line 145-155: Update BuildLetterbox so a null result from src.Resize
throws an exception instead of skipping canvas.DrawBitmap and returning a blank
LetterboxGray bitmap; preserve the existing draw path for successful resizes so
the caller’s failure handling is triggered.
In `@server/src/Services/TypesettingService.cs`:
- Around line 151-178: Fix WhiteFillWithMask by disposing the original
SKBitmap.Decode(imagePng) result separately while retaining the copied BGRA
bitmap. Require decoded.Resize to succeed when dimensions differ; if it returns
null, return the original image instead of using a mismatched mask. Update the
unsafe pixel traversal to advance each row using bitmap.RowBytes and
mask.RowBytes, and process pixels by their actual row offsets rather than
assuming tightly packed rows.
- Line 204: Update the PNG encoding flow in RunInpaintAsync to check both the
Cv2.ImEncode success flag and encoded-buffer non-emptiness, returning the
original imagePng when encoding fails or produces no data. Correct the dilate
parameter documentation to describe it as the structuring-element size, or
change the Size(dilate, dilate) construction to use 2 * dilate + 1 when dilate
is intended as a radius.
---
Outside diff comments:
In `@server/src/Services/InpaintService.cs`:
- Around line 400-411: Remove the stale CopyRegion summary immediately before
MaskComposite, leaving only MaskComposite’s mask-compositing documentation
there. Restore that rectangular-copy summary directly above the CopyRegion
method declaration so each method has exactly its own documentation.
---
Nitpick comments:
In `@server/src/Routes/PortalRoutes.cs`:
- Around line 173-176: Update the MapDelete route for
jobs/{id}/textseg-blocks/{index} and the corresponding sibling route to
constrain the parameter as {index:int}, matching the existing bubbleIndex route
pattern. Return the parsed JSON elements directly from the handler and remove
the redundant Deserialize<object>/re-serialization step and its unused
naming-policy handling.
- Around line 137-141: Make cached JSON writes in the page translation and
block-deletion flows atomic and safe under concurrent GET requests. Reuse the
existing per-job lock when writing textseg_blocks.json, or write the complete
content to a temporary file and atomically replace the cache file; ensure
PortalRoutes’ cache read never observes partial JSON or fails due to parallel
writes.
In `@server/src/Services/PageTranslationService.cs`:
- Around line 767-778: Extract the shared centre-containment logic from
GetTypesettingBox and IsInsideAnyBubble into one helper, such as CenterInside,
and reuse it in both methods. Update LoadTextSegMaskAsync to accept a
CancellationToken and propagate the existing ct from both call sites through its
async operations.
In `@server/src/Services/TextSegmentationService.cs`:
- Around line 46-58: Update TextSegmentationService.LoadModel to create and
fully initialize a new InferenceSession and input name in local variables before
publishing them to the service fields. Dispose the existing _session before
replacing it, and assign _inputName before _session so IsReady cannot observe a
partially initialized model.
- Around line 115-122: In the crop calculation, replace the single mScale used
by the crop block with independent width and height scales derived from mW and
mH relative to ModelInput. Apply the width scale to padLeft and newW, and the
height scale to padTop and newH, preserving the existing rounding and clamping
behavior in cLeft, cTop, cW, and cH.
- Around line 94-110: Update the mask-selection logic in the segmentation output
handling to resolve the mask tensor by its ONNX output name first, rather than
assuming a positional index. Retain channel-count detection as the fallback when
names do not identify the mask, and remove the unsafe fallback that can select
lines_map; continue throwing when no usable mask output is available.
- Around line 191-214: Update BuildBinaryMask to accept the DenseTensor<float>
output from AsTensor<float>, validate that IsReversedStride is false, and read
values from Buffer.Span using the y * w + x offset instead of multidimensional
indexing. Avoid ToDenseTensor and preserve the existing thresholding and
bitmap-writing behavior.
🪄 Autofix
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: d6197559-78b3-45d9-8ac5-b977e4f8fd3c
⛔ 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 (18)
extension/package.jsonextension/static/manifest.jsonserver/ClientApp/src/api.tsserver/ClientApp/src/components/BubbleCanvas.tsxserver/ClientApp/src/pages/StudioPage.tsxserver/server-csharp.csprojserver/src/BootExtensions.csserver/src/Config.csserver/src/ModelSettingsStore.csserver/src/Routes/HealthRoutes.csserver/src/Routes/PortalRoutes.csserver/src/ServiceExtensions.csserver/src/Services/InpaintService.csserver/src/Services/PageTranslationService.csserver/src/Services/TextSegmentationService.csserver/src/Services/TypesettingService.csserver/src/Workers/InferenceJob.csserver/src/Workers/InferenceWorker.cs
Comment on lines
+874
to
+896
| <div | ||
| onClick={() => setSelectedTextSegIndex(isSelected() ? null : i())} | ||
| class={`group flex cursor-pointer items-center gap-1 border-b border-slate-100 px-2 py-1.5 transition-colors ${ | ||
| isSelected() | ||
| ? "border-l-2 border-l-orange-400 bg-orange-50" | ||
| : "hover:bg-slate-50" | ||
| }`} | ||
| > | ||
| <span class={`font-mono text-[10px] font-medium shrink-0 ${isSelected() ? "text-orange-600" : "text-slate-400"}`}> | ||
| #{i()} | ||
| </span> | ||
| <span class="flex-1 truncate text-[10px] text-slate-500"> | ||
| {box.w}×{box.h} @ {box.x},{box.y} | ||
| </span> | ||
| <button | ||
| onClick={(e) => { e.stopPropagation(); void handleDeleteTextSeg(i()); }} | ||
| class="invisible shrink-0 rounded p-0.5 text-slate-400 hover:bg-red-50 hover:text-red-500 group-hover:visible" | ||
| title="Delete TextSeg block" | ||
| aria-label="Delete TextSeg block" | ||
| > | ||
| <Trash2 class="h-3 w-3" /> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Make the TextSeg block row keyboard accessible.
The row is a div with an onClick handler. Keyboard users cannot select a block, and screen readers do not announce it as interactive. The Stage 3 overlay list at line 930 uses a <button> for the same interaction. Biome also lints a11y in this repo, as shown by the biome-ignore lint/a11y/useKeyWithMouseEvents comment in server/ClientApp/src/components/BubbleCanvas.tsx at line 514, so this row will likely fail the lint step.
Restructure the row so the selectable area is a <button> and the delete control is a sibling, not a descendant. Nesting a button inside a button is invalid HTML.
🐛 Proposed fix
- <div
- onClick={() => setSelectedTextSegIndex(isSelected() ? null : i())}
- class={`group flex cursor-pointer items-center gap-1 border-b border-slate-100 px-2 py-1.5 transition-colors ${
+ <div
+ class={`group flex items-center gap-1 border-b border-slate-100 px-2 py-1.5 transition-colors ${
isSelected()
? "border-l-2 border-l-orange-400 bg-orange-50"
: "hover:bg-slate-50"
}`}
>
- <span class={`font-mono text-[10px] font-medium shrink-0 ${isSelected() ? "text-orange-600" : "text-slate-400"}`}>
- #{i()}
- </span>
- <span class="flex-1 truncate text-[10px] text-slate-500">
- {box.w}×{box.h} @ {box.x},{box.y}
- </span>
+ <button
+ type="button"
+ onClick={() => setSelectedTextSegIndex(isSelected() ? null : i())}
+ aria-pressed={isSelected()}
+ class="flex min-w-0 flex-1 cursor-pointer items-center gap-1 text-left"
+ >
+ <span class={`font-mono text-[10px] font-medium shrink-0 ${isSelected() ? "text-orange-600" : "text-slate-400"}`}>
+ #{i()}
+ </span>
+ <span class="flex-1 truncate text-[10px] text-slate-500">
+ {box.w}×{box.h} @ {box.x},{box.y}
+ </span>
+ </button>
<button
- onClick={(e) => { e.stopPropagation(); void handleDeleteTextSeg(i()); }}
+ type="button"
+ onClick={() => void handleDeleteTextSeg(i())}
class="invisible shrink-0 rounded p-0.5 text-slate-400 hover:bg-red-50 hover:text-red-500 group-hover:visible"
title="Delete TextSeg block"
aria-label="Delete TextSeg block"
>
<Trash2 class="h-3 w-3" />
</button>
</div>📝 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
| <div | |
| onClick={() => setSelectedTextSegIndex(isSelected() ? null : i())} | |
| class={`group flex cursor-pointer items-center gap-1 border-b border-slate-100 px-2 py-1.5 transition-colors ${ | |
| isSelected() | |
| ? "border-l-2 border-l-orange-400 bg-orange-50" | |
| : "hover:bg-slate-50" | |
| }`} | |
| > | |
| <span class={`font-mono text-[10px] font-medium shrink-0 ${isSelected() ? "text-orange-600" : "text-slate-400"}`}> | |
| #{i()} | |
| </span> | |
| <span class="flex-1 truncate text-[10px] text-slate-500"> | |
| {box.w}×{box.h} @ {box.x},{box.y} | |
| </span> | |
| <button | |
| onClick={(e) => { e.stopPropagation(); void handleDeleteTextSeg(i()); }} | |
| class="invisible shrink-0 rounded p-0.5 text-slate-400 hover:bg-red-50 hover:text-red-500 group-hover:visible" | |
| title="Delete TextSeg block" | |
| aria-label="Delete TextSeg block" | |
| > | |
| <Trash2 class="h-3 w-3" /> | |
| </button> | |
| </div> | |
| <div | |
| class={`group flex items-center gap-1 border-b border-slate-100 px-2 py-1.5 transition-colors ${ | |
| isSelected() | |
| ? "border-l-2 border-l-orange-400 bg-orange-50" | |
| : "hover:bg-slate-50" | |
| }`} | |
| > | |
| <button | |
| type="button" | |
| onClick={() => setSelectedTextSegIndex(isSelected() ? null : i())} | |
| aria-pressed={isSelected()} | |
| class="flex min-w-0 flex-1 cursor-pointer items-center gap-1 text-left" | |
| > | |
| <span class={`font-mono text-[10px] font-medium shrink-0 ${isSelected() ? "text-orange-600" : "text-slate-400"}`}> | |
| #{i()} | |
| </span> | |
| <span class="flex-1 truncate text-[10px] text-slate-500"> | |
| {box.w}×{box.h} @ {box.x},{box.y} | |
| </span> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={() => void handleDeleteTextSeg(i())} | |
| class="invisible shrink-0 rounded p-0.5 text-slate-400 hover:bg-red-50 hover:text-red-500 group-hover:visible" | |
| title="Delete TextSeg block" | |
| aria-label="Delete TextSeg block" | |
| > | |
| <Trash2 class="h-3 w-3" /> | |
| </button> | |
| </div> |
🤖 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 874 - 896,
Restructure the TextSeg row around the existing selection handler so the
selectable area is a button, making it keyboard-accessible and announced as
interactive. Move the delete button currently inside the row to be a sibling
outside the selectable button, while preserving event propagation prevention,
styling, and selection behavior without nesting buttons.
Comment on lines
+36
to
+37
| <PackageReference Include="OpenCvSharp4" Version="4.10.0.20241108" /> | ||
| <PackageReference Include="OpenCvSharp4.official.runtime.linux-x64" Version="4.13.0.20260627" /> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check published OpenCvSharp package versions and find usage in the repository.
set -uo pipefail
for pkg in opencvsharp4 opencvsharp4.official.runtime.linux-x64; do
echo "=== $pkg ==="
curl -s "https://api.nuget.org/v3-flatcontainer/${pkg}/index.json" | jq -r '.versions[-15:][]'
done
# Where is OpenCV actually used?
rg -n --type=cs -C3 'OpenCvSharp|Cv2\.'Repository: deckyfx/web-ocr
Length of output: 702
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== project file ==="
cat -n server/server-csharp.csproj | sed -n '25,45p'
echo "=== C# files and OpenCvSharp references ==="
git ls-files '*.cs' | while IFS= read -r f; do
if rg -n -C3 'OpenCvSharp|Cv2\.' "$f"; then
:
fi
done
echo "=== package metadata ==="
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
for spec in \
"opencvsharp4/4.10.0.20241108" \
"opencvsharp4/4.13.0.20260627" \
"opencvsharp4.official.runtime.linux-x64/4.13.0.20260627"
do
pkg="${spec%/*}"
ver="${spec#*/}"
url="https://api.nuget.org/v3-flatcontainer/${pkg}/${ver}/${pkg}.${ver}.nupkg"
out="$tmpdir/${pkg}.${ver}.nupkg"
echo "--- $pkg $ver ---"
curl -fsSL "$url" -o "$out"
unzip -l "$out" | sed -n '1,80p'
unzip -p "$out" '*.nuspec' | sed -n '1,160p'
doneRepository: deckyfx/web-ocr
Length of output: 13863
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== target framework and runtime identifiers ==="
cat -n server/server-csharp.csproj | sed -n '1,28p'
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
download_pkg() {
local pkg="$1" ver="$2"
local lower_pkg="${pkg,,}"
local out="$tmpdir/${lower_pkg}.${ver}.nupkg"
curl -fsSL "https://api.nuget.org/v3-flatcontainer/${lower_pkg}/${ver}/${lower_pkg}.${ver}.nupkg" -o "$out"
mkdir -p "$tmpdir/${lower_pkg}.${ver}"
unzip -q "$out" -d "$tmpdir/${lower_pkg}.${ver}"
}
echo "=== available 4.10 runtime versions ==="
curl -fsSL 'https://api.nuget.org/v3-flatcontainer/opencvsharp4.official.runtime.linux-x64/index.json' |
jq -r '.versions[]' | grep '^4\.10\.'
download_pkg 'OpenCvSharp4.official.runtime.linux-x64' '4.10.0.20241108'
download_pkg 'OpenCvSharp4.official.runtime.linux-x64' '4.13.0.20260627'
echo "=== native library exports: matching 4.10 versus selected 4.13 ==="
for f in \
"$tmpdir"/opencvsharp4.official.runtime.linux-x64.4.10.0.20241108/runtimes/linux-x64/native/libOpenCvSharpExtern.so \
"$tmpdir"/opencvsharp4.official.runtime.linux-x64.4.13.0.20260627/runtimes/linux-x64/native/libOpenCvSharpExtern.so
do
echo "--- $f ---"
file "$f"
nm -D --defined-only "$f" | awk '{print $3}' | sort > "$f.exports"
echo "export count: $(wc -l < "$f.exports")"
done
comm -3 \
"$tmpdir"/opencvsharp4.official.runtime.linux-x64.4.10.0.20241108/runtimes/linux-x64/native/libOpenCvSharpExtern.so.exports \
"$tmpdir"/opencvsharp4.official.runtime.linux-x64.4.13.0.20260627/runtimes/linux-x64/native/libOpenCvSharpExtern.so.exports |
sed -n '1,120p'
echo "=== symbols used by the 4.10 managed assembly for this code path ==="
download_pkg 'OpenCvSharp4' '4.10.0.20241108'
strings "$tmpdir"/opencvsharp4.4.10.0.20241108/lib/net6.0/OpenCvSharp.dll |
grep -E 'core_.*(imdecode|rectangle|inpaint|imencode|countNonZero|dilate|getStructuringElement)|OpenCvSharpExtern' |
sort -uRepository: deckyfx/web-ocr
Length of output: 2033
🏁 Script executed:
#!/bin/bash
set -u
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
download_pkg() {
local pkg="$1" ver="$2"
local lower_pkg="${pkg,,}"
local out="$tmpdir/${lower_pkg}.${ver}.nupkg"
curl -fsSL "https://api.nuget.org/v3-flatcontainer/${lower_pkg}/${ver}/${lower_pkg}.${ver}.nupkg" -o "$out" || return 1
mkdir -p "$tmpdir/${lower_pkg}.${ver}"
unzip -q "$out" -d "$tmpdir/${lower_pkg}.${ver}" || return 1
}
download_pkg 'OpenCvSharp4.official.runtime.linux-x64' '4.10.0.20241108'
download_pkg 'OpenCvSharp4.official.runtime.linux-x64' '4.13.0.20260627'
download_pkg 'OpenCvSharp4' '4.10.0.20241108'
echo "=== native export comparison ==="
for ver in 4.10.0.20241108 4.13.0.20260627; do
f="$tmpdir/opencvsharp4.official.runtime.linux-x64.$ver/runtimes/linux-x64/native/libOpenCvSharpExtern.so"
echo "--- $ver ---"
if command -v nm >/dev/null 2>&1; then
nm -D --defined-only "$f" | awk '{print $3}' | sort -u > "$tmpdir/$ver.exports"
else
readelf -Ws "$f" | awk '$4 == "FUNC" && $7 != "UND" {print $8}' | sort -u > "$tmpdir/$ver.exports"
fi
wc -l "$tmpdir/$ver.exports"
done
echo "--- symbols only in one version (first 160) ---"
comm -3 "$tmpdir/4.10.0.20241108.exports" "$tmpdir/4.13.0.20260627.exports" | sed -n '1,160p'
echo "=== managed wrapper strings relevant to the repository call path ==="
strings "$tmpdir/opencvsharp4.4.10.0.20241108/lib/net6.0/OpenCvSharp.dll" |
grep -Ei 'OpenCvSharpExtern|inpaint|imdecode|imencode|rectangle|countnonzero|dilate|getstructuringelement' |
sort -u | sed -n '1,160p'Repository: deckyfx/web-ocr
Length of output: 4029
🏁 Script executed:
#!/bin/bash
set -u
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
download_pkg() {
local pkg="$1" ver="$2"
local id="${pkg,,}"
local nupkg="$tmpdir/$id.$ver.nupkg"
curl -fsSL "https://api.nuget.org/v3-flatcontainer/$id/$ver/$id.$ver.nupkg" -o "$nupkg" || return 1
mkdir -p "$tmpdir/$id.$ver"
unzip -q "$nupkg" -d "$tmpdir/$id.$ver" || return 1
}
download_pkg opencvsharp4 4.10.0.20241108
download_pkg opencvsharp4.official.runtime.linux-x64 4.10.0.20241108
download_pkg opencvsharp4.official.runtime.linux-x64 4.13.0.20260627
echo "=== runtime README and package references ==="
for f in \
"$tmpdir"/opencvsharp4.official.runtime.linux-x64.4.10.0.20241108/README.runtime.md \
"$tmpdir"/opencvsharp4.official.runtime.linux-x64.4.13.0.20260627/README.runtime.md
do
echo "--- $f ---"
sed -n '1,180p' "$f"
done
echo "=== OpenCvSharp bridge symbols in native libraries ==="
for ver in 4.10.0.20241108 4.13.0.20260627; do
f="$tmpdir/opencvsharp4.official.runtime.linux-x64.$ver/runtimes/linux-x64/native/libOpenCvSharpExtern.so"
echo "--- $ver ---"
nm -D --defined-only "$f" 2>/dev/null |
awk '{print $3}' |
grep -E '^(core|imgcodecs|imgproc|photo)_' |
grep -Ei 'inpaint|imdecode|imencode|rectangle|count.?non.?zero|dilate|getstructuringelement' |
sort -u
done
echo "=== relevant managed assembly strings ==="
python3 - "$tmpdir/opencvsharp4.4.10.0.20241108/lib/net6.0/OpenCvSharp.dll" <<'PY'
import re, sys
data = open(sys.argv[1], "rb").read()
strings = sorted(set(x.decode("ascii", "ignore") for x in re.findall(rb"[\x20-\x7e]{4,}", data)))
for s in strings:
if ("OpenCvSharpExtern" in s or
any(k in s.lower() for k in ("inpaint", "imdecode", "imencode", "rectangle", "countnonzero", "dilate", "getstructuringelement"))):
print(s)
PY
echo "=== repository platform/deployment references ==="
git ls-files | grep -E '(^|/)(README|Dockerfile|.*\.yml$|.*\.yaml$|.*\.csproj$)' |
while IFS= read -r f; do
rg -n -i -C2 'linux|windows|macos|macOS|runtime|platform|decky|steam deck' "$f" || true
doneRepository: deckyfx/web-ocr
Length of output: 6857
🏁 Script executed:
#!/bin/bash
set -u
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL \
'https://api.nuget.org/v3-flatcontainer/opencvsharp4/4.10.0.20241108/opencvsharp4.10.0.20241108.nupkg' |
unzip -p - 'lib/net6.0/OpenCvSharp.dll' > "$tmpdir/OpenCvSharp.dll"
echo "=== managed assembly strings for the inpainting call path ==="
python3 - "$tmpdir/OpenCvSharp.dll" <<'PY'
import re, sys
data = open(sys.argv[1], "rb").read()
items = sorted(set(x.decode("ascii", "ignore")
for x in re.findall(rb"[\x20-\x7e]{4,}", data)))
needles = ("OpenCvSharpExtern", "inpaint", "imdecode", "imencode",
"rectangle", "countnonzero", "dilate", "getstructuringelement")
for item in items:
if any(needle in item.lower() for needle in needles):
print(item)
PY
echo "=== repository files and platform statements ==="
git ls-files | sed -n '1,120p'
rg -n -i -C2 'linux|windows|macos|macOS|steam deck|decky|platform|deployment|supported' \
--glob '!server/server-csharp.csproj' . || trueRepository: deckyfx/web-ocr
Length of output: 50374
Add native runtimes for supported operating systems.
The server’s documented targets include Windows and macOS, but this project references only the Linux x64 runtime. Add the required Windows and macOS runtime packages so OpenCV calls do not fail on those platforms.
🤖 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/server-csharp.csproj` around lines 36 - 37, Add the OpenCvSharp native
runtime package references for the supported Windows and macOS platforms
alongside OpenCvSharp4 and the existing Linux x64 runtime in the project
configuration. Use the appropriate Windows and macOS runtime package variants
while preserving the current Linux runtime reference.
5 tasks
Comments 1-3 (StudioPage.tsx):
- Surface TextSeg load failure via setActionError in catch block
- Replace div row with button for keyboard accessibility
- Add group-focus-within:visible / focus-visible:visible to delete button
Comment 4 (server-csharp.csproj):
- Add OpenCvSharp4.runtime.win for Windows cross-platform support
(macOS arm64 package not yet published by schimatk)
Comment 5 (BootExtensions.cs):
- Wrap DownloadTextSegAsync body in try/catch so a download failure
is non-fatal (logged as warning, server continues without TextSeg)
Comments 8-9 (PageTranslationService.cs):
- Group TextSeg blocks by typesetting target bubble before OCR/translate
using BubbleBoxComparer to prevent duplicate overlapping translations
- Add LoadCachedTextSegAsync that loads both mask PNG + textseg_blocks.json
so Telea pass receives block list on re-render/re-inpaint paths
Comments 10-13 (various services):
- Guard LaMa path on empty TextSeg mask (preSeg?.Mask is {Length>0})
- Throw InvalidOperationException on null resize in TextSeg BuildLetterbox
- Fix WhiteFillWithMask bitmap leak + unsafe RowBytes pointer walk
- Check Cv2.ImEncode return value in TeleaInpaintBlocks; fix doc comment
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
IsInsideAnyBubble()centre-point test separates bubble vs non-bubble blocksMaskCompositefix inInpaintPageWithPixelMask: previousCopyRegionoverwrote the entire bubble bounding box (destroying bubble outlines and artwork); now only writes pixels whererawMaskistrue, preserving all non-text pixelsTest plan
/healthcorrectly reportsTextSegReadystate🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores