ADFA-3290 | Implement smart boundary detection for dynamic margins#1171
Open
ADFA-3290 | Implement smart boundary detection for dynamic margins#1171
Conversation
jatezzz
requested review from
a team,
Daniel-ADFA,
avestaadfa,
dara-abijo-adfa and
hal-eisen-adfa
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 Walkthrough
WalkthroughImage analysis moved off the main thread and now computes smart left/right guides by adding a vertical projection utility and a SmartBoundaryDetector; the ViewModel uses the detector on the rotated bitmap and converts detected pixel bounds into guide percentages, replacing fixed default guides. Changes
Sequence DiagramsequenceDiagram
actor User
participant ViewModel as ComputerVisionViewModel
participant Default as Dispatchers.Default
participant Detector as SmartBoundaryDetector
participant Utils as BitmapUtils
participant Bitmap as Bitmap
User->>ViewModel: selectImage()
ViewModel->>Default: withContext(Dispatchers.Default) - decode & rotate
Default->>Bitmap: decode / apply rotation
Bitmap-->>Default: rotatedBitmap
Default->>Detector: detectSmartBoundaries(rotatedBitmap)
Detector->>Utils: calculateVerticalProjection(rotatedBitmap)
Utils->>Bitmap: readPixels()
Bitmap-->>Utils: pixel data
Utils->>Utils: compute vertical edge projection
Utils-->>Detector: FloatArray projection
Detector->>Detector: analyze left/right zones, find gap midpoints
Detector-->>Default: Pair(leftPixel, rightPixel)
Default->>ViewModel: convert to percentages & update guides
ViewModel->>ViewModel: emit UI state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/SmartBoundaryDetector.kt (1)
11-11: Minor type inconsistency.
MIN_GAP_WIDTH_PERCENTis declared asDouble(0.02) while other percentage constants areFloat(e.g., 0.05f). Consider using0.02ffor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/SmartBoundaryDetector.kt` at line 11, MIN_GAP_WIDTH_PERCENT is declared with a Double literal (0.02) while other percentage constants use Float; change the literal to 0.02f so MIN_GAP_WIDTH_PERCENT is a Float consistent with the other constants in SmartBoundaryDetector.kt (update the constant declaration for MIN_GAP_WIDTH_PERCENT to use the f-suffix).cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt (1)
116-120: Consider validating thatleftPct < rightPctand values are in valid range.While
SmartBoundaryDetectoris designed to return sensible bounds, there's no explicit validation thatleftPct < rightPctor that values fall within[0, 1]. The downstreamGuidelinesView.updateGuidelines()(per the relevant code snippet) has no bounds checks either.For robustness, consider adding a sanity check:
🛡️ Proposed validation
val widthFloat = rotatedBitmap.width.toFloat() - val leftPct = leftBoundPx / widthFloat - val rightPct = rightBoundPx / widthFloat + val rawLeftPct = (leftBoundPx / widthFloat).coerceIn(0f, 1f) + val rawRightPct = (rightBoundPx / widthFloat).coerceIn(0f, 1f) + val leftPct = minOf(rawLeftPct, rawRightPct) + val rightPct = maxOf(rawLeftPct, rawRightPct) Triple(rotatedBitmap, leftPct, rightPct)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt` around lines 116 - 120, Validate and sanitize the bounds returned by SmartBoundaryDetector.detectSmartBoundaries before calling GuidelinesView.updateGuidelines: compute leftPct/rightPct from rotatedBitmap as shown, then ensure they are within [0.0, 1.0] (clamp) and that leftPct < rightPct (swap or adjust if not), and log a warning when you modify values; pass the sanitized leftPct/rightPct to GuidelinesView.updateGuidelines to guarantee valid input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/SmartBoundaryDetector.kt`:
- Around line 75-92: The loop in SmartBoundaryDetector misses leading gaps
because currentGapStart is only set on transitions from active to inactive; to
fix, initialize gap detection for leading inactive samples before iterating
(e.g., if signal is not empty and signal[0] <= threshold set currentGapStart =
0) or add a check inside the loop to set currentGapStart when index == 0 and
!isActive; keep the existing gap-closing logic (currentGapStart, maxGapLength,
maxGapMidpoint) unchanged so leading gaps are measured and recorded
consistently.
- Around line 25-36: Guard against invalid ranges before calling
projection.copyOfRange: check that ignoredEdgePixels < leftZoneEnd before
building leftSignal and that rightZoneStart < rightZoneEnd before building
rightSignal; if a range would be empty or invalid (e.g., due to small image or
large edgeIgnorePercent/LEFT_ZONE_END_PERCENT/RIGHT_ZONE_START_PERCENT), either
adjust the start/end to valid bounds or skip creating the signal and handle the
fallback (call findBestGapMidpoint with an empty/normalized signal or return
nulls) so projection.copyOfRange cannot throw; update the logic around
ignoredEdgePixels, leftZoneEnd, rightZoneStart, rightZoneEnd, leftSignal,
rightSignal and subsequent findBestGapMidpoint/leftBound/rightBound handling to
safely handle these edge cases.
---
Nitpick comments:
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.kt`:
- Around line 116-120: Validate and sanitize the bounds returned by
SmartBoundaryDetector.detectSmartBoundaries before calling
GuidelinesView.updateGuidelines: compute leftPct/rightPct from rotatedBitmap as
shown, then ensure they are within [0.0, 1.0] (clamp) and that leftPct <
rightPct (swap or adjust if not), and log a warning when you modify values; pass
the sanitized leftPct/rightPct to GuidelinesView.updateGuidelines to guarantee
valid input.
In
`@cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/SmartBoundaryDetector.kt`:
- Line 11: MIN_GAP_WIDTH_PERCENT is declared with a Double literal (0.02) while
other percentage constants use Float; change the literal to 0.02f so
MIN_GAP_WIDTH_PERCENT is a Float consistent with the other constants in
SmartBoundaryDetector.kt (update the constant declaration for
MIN_GAP_WIDTH_PERCENT to use the f-suffix).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6c44ef6-e502-44d4-9810-d8bdf0693a3f
📒 Files selected for processing (3)
cv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/ui/viewmodel/ComputerVisionViewModel.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/BitmapUtils.ktcv-image-to-xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/SmartBoundaryDetector.kt
...xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/SmartBoundaryDetector.kt
Show resolved
Hide resolved
...xml/src/main/java/org/appdevforall/codeonthego/computervision/utils/SmartBoundaryDetector.kt
Show resolved
Hide resolved
Replaces hardcoded bounds with a vertical projection edge-detection algorithm.
jatezzz
force-pushed
the
feat/ADFA-3290-smart-boundary-detection-experimental
branch
from
c689644 to
cb36d75
Compare
hal-eisen-adfa
approved these changes
Apr 10, 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
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.
Description
This PR replaces the hardcoded margin boundaries (0.2f and 0.8f) with a smart algorithm that calculates their optimal positions dynamically. It introduces the
SmartBoundaryDetectorand a newcalculateVerticalProjectionutility inBitmapUtilsto analyze image content and snap guidelines to the best available vertical gaps.Details
BitmapUtils.calculateVerticalProjection.SmartBoundaryDetectorto find gap midpoints using the projection data.ComputerVisionViewModel.loadImageFromUrito apply the dynamically calculated left and right percentage bounds instead of resetting to default hardcoded values.document_5098099422705747202.mp4
Ticket
ADFA-3290
Observation
The edge detection approach in
calculateVerticalProjectionuses a simplified, highly performant Sobel-like technique by directly calculating pixel variations on the red channel, avoiding heavy matrix operations on the main thread.