Skip to content

Conversation

@bhavul
Copy link
Contributor

@bhavul bhavul commented Nov 9, 2025

This is based on discussion in #140.

Summary

This PR adds a Smart Lasso Selection feature that allows users to select content by drawing a closed loop with their pen/stylus, making selection much more intuitive for EMR device users. The feature intelligently detects closed loops and shows the selection panel, with smart fallback to drawing if the user dismisses without taking action.

How does this work?

  • Added an algorithm to detect closed loop (few parameters can be optimised further)
  • Fallback: If user dismisses selection without using tools, the original stroke is drawn
  • Settings toggle: Feature is OFF by default, can be enabled in Settings > General

Note: Scribble-to-erase takes priority, so it doesn't interfere with normal drawing.

User Flow

  1. User draws closed loop with pen
  2. If loop contains content → Shows selection panel
  3. User chooses:
    a) Use tool (cut/copy/paste/duplicate) → Content is selected/manipulated
    b) Tap outside → Original loop stroke is drawn on page

Closed loop detection algorithm

It uses several heuristics to ensure accurate recognition:

  1. Minimum points: Requires ≥20 points (avoids tiny accidental loops)
  2. Closure check: First and last points must be within 15% of bounding box diagonal
  3. Minimum perimeter: ≥100 pixels to filter out tiny loops
  4. Reversal limit: Allows natural hand movement but rejects scribbles (max 8 reversals)
  5. Content check: Only triggers if strokes/images are actually inside the loop

Here's a video showcasing the feature and settings: https://drive.google.com/file/d/17nynCt7uNkB5glhnBmw1N1AGgTBBcnRe/view?usp=sharing

Maybe in Future?

Allow tuning detection thresholds in advanced settings

@Ethran Ethran added the enhancement New feature or request label Nov 9, 2025
@Ethran Ethran requested a review from Copilot November 9, 2025 21:20
@Ethran
Copy link
Owner

Ethran commented Nov 9, 2025

Let copilot review it first, maybe it will find some problems. I will look at it latter.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request adds a "smart lasso" selection feature that allows users to draw a closed loop to select content. When enabled, users can dismiss the selection to fall back to drawing the original stroke, providing a more intuitive drawing/selection workflow.

  • Adds a new smartLassoEnabled setting to app configuration
  • Implements smart lasso selection detection in drawing logic
  • Adds fallback mechanism to draw original stroke when selection is dismissed
  • Enhances JSON deserialization for backward compatibility

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
app/src/main/res/values/strings.xml Added English string resource for smart lasso setting
app/src/main/res/values-pl/strings.xml Added Polish translation for smart lasso setting
app/src/main/java/com/ethran/notable/ui/views/Settings.kt Added UI toggle for smart lasso feature in settings
app/src/main/java/com/ethran/notable/editor/ui/SelectorBitmap.kt Refactored selection dismissal to use centralized control tower method
app/src/main/java/com/ethran/notable/editor/state/SelectionState.kt Added state tracking for pending smart lasso strokes
app/src/main/java/com/ethran/notable/editor/EditorControlTower.kt Implemented dismissSelection method with smart lasso fallback logic
app/src/main/java/com/ethran/notable/editor/DrawCanvas.kt Integrated smart lasso detection in drawing input handling
app/src/main/java/com/ethran/notable/data/db/Kv.kt Added JSON configuration for backward compatibility
app/src/main/java/com/ethran/notable/data/datastore/AppSettings.kt Added smartLassoEnabled field to app settings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Ethran Ethran requested a review from Copilot November 9, 2025 21:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bhavul
Copy link
Contributor Author

bhavul commented Nov 10, 2025

@Ethrlet you wanna have a look now?

@Ethran
Copy link
Owner

Ethran commented Nov 10, 2025

I will look into it this week, I need some more time to do it.

Copy link
Owner

@Ethran Ethran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some unnecessary operations in your implementation, please be mindful of what you are calculating before drawing strokes on the screen.

History should handle uncommitted strokes.

Don't duplicate existing functions, if something is implemented already, use it.

The EditorControlTower will change a little bit, mind this doing changes. 57d5fe2

Comment on lines +267 to +303
Log.i("SmartLasso", "User dismissed smart lasso selection, drawing the original stroke with original pen settings")
// User dismissed without using the panel, so draw the original stroke with original settings
// Save the pending data before reset clears it
val strokeToDrawCopy = pendingStroke.toList()
val penCopy = pendingPen
val strokeSizeCopy = pendingStrokeSize
val colorCopy = pendingColor

// Reset the selection
state.selectionState.reset()

// Now draw the pending stroke with its original pen settings
val strokeHistoryBatch = mutableListOf<String>()
com.ethran.notable.editor.utils.handleDraw(
page,
strokeHistoryBatch,
strokeSizeCopy,
colorCopy,
penCopy,
strokeToDrawCopy
)

// Add to history
if (strokeHistoryBatch.isNotEmpty()) {
history.addOperationsToHistory(
operations = listOf(
Operation.DeleteStroke(strokeHistoryBatch)
)
)
}

state.isDrawing = true

// Refresh UI
scope.launch {
DrawCanvas.refreshUi.emit(Unit)
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be moved to separate funtion

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it should be done as new operation list in History(pageView: PageView),

  private var uncommittedOperations: OperationList = mutableListOf()

And there should be functions:
commitOperations()
addToUncommittedOperations(…)
clearUncommittedOperations(…)

(There probably are better names for them)

See also branch dev, I made recently some changes how the history is handled (I needed to handled one thing differently, and ended up changing a lot):
Commit: 57d5fe2
It will be merged when I address most of TODO's in this marge request. #156

private val kvRepository = KvRepository(context)

// Configure JSON to ignore unknown keys for backward compatibility
private val json = Json { ignoreUnknownKeys = true }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to check I really want to enable it. Having it set to false makes sure that I know exact state of app config. For now this PR should not make this change.

private fun distance(p1: StrokePoint, p2: StrokePoint): Float {
val dx = p1.x - p2.x
val dy = p1.y - p2.y
return sqrt(dx * dx + dy * dy)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use sqrt. It adds unnecessary computations, and its enough to use Manhattan metric. See scribble to erase.

val boxHeight = boundingBox.height()

// Calculate diagonal of bounding box
val diagonal = sqrt(boxWidth * boxWidth + boxHeight * boxHeight)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, use Manhattan metric.

/**
* Calculates the Euclidean distance between two points
*/
private fun distance(p1: StrokePoint, p2: StrokePoint): Float {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as distance might will be used in multiple parts of project it should be moved to editor/utils/…

/**
* Calculates the total perimeter length of a stroke path
*/
private fun calculatePerimeter(points: List<StrokePoint>): Float {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its just a path length, not a Perimeter. Also, reuse function from scribble to erase.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it seems as a quite bad way to determine if it is the loop. I would propose to select some points on the path, and check if distance between them is sufficiently large. ie let those points divide path in roughly 4 parts, and calculate distance between those 3 points. BUT be careful not to do too many operations, check if choosing base on index in table is good enough, or base on time (binary search).

* Calculates direction reversals but with different thresholds for lasso detection
* Unlike scribble detection, we're looking for smooth loops, not back-and-forth motion
*/
private fun calculateNumReversalsForLasso(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why it is separate function from what is used for scribble to ease?

Comment on lines +171 to +174
editorState.selectionState.pendingSmartLassoStroke = touchPoints
editorState.selectionState.pendingSmartLassoPen = editorState.pen
editorState.selectionState.pendingSmartLassoStrokeSize = editorState.penSettings[editorState.pen.penName]?.strokeSize
editorState.selectionState.pendingSmartLassoColor = editorState.penSettings[editorState.pen.penName]?.color
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't good way of storing this information. It should be stored the same as stack for undo/redo.

Comment on lines +40 to +45
// Smart lasso: stores the original stroke data if selection was made via smart lasso
// This allows fallback to drawing the stroke if user dismisses without using the panel
var pendingSmartLassoStroke by mutableStateOf<List<StrokePoint>?>(null)
var pendingSmartLassoPen by mutableStateOf<Pen?>(null)
var pendingSmartLassoStrokeSize by mutableStateOf<Float?>(null)
var pendingSmartLassoColor by mutableStateOf<Int?>(null)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the place for those, see other comments.

// Additional check: ensure the stroke doesn't have too many sharp reversals
// (which would indicate scribbling rather than deliberate loop drawing)
// We allow some reversals (unlike scribble which requires many), but not too many
val reversals = calculateNumReversalsForLasso(points)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to calculate it again? We already calculate it in scribble to erase, if both are turned on, then we do unnecessary calculations. Same for path lengths.

@bhavul
Copy link
Contributor Author

bhavul commented Feb 10, 2026

Hi @Ethran Life's gotten busy, I'm afraid. I've also stopped using Notable for the most part recently (no major reason).
So I'll pass on updating this PR further. Feel free to close it or resume work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants