Skip to content

PAINTROID-796 Add Clip area tool - #132

Open
Amit-Matth wants to merge 13 commits into
Catrobat:developfrom
Amit-Matth:PAINTROID-796
Open

PAINTROID-796 Add Clip area tool#132
Amit-Matth wants to merge 13 commits into
Catrobat:developfrom
Amit-Matth:PAINTROID-796

Conversation

@Amit-Matth

@Amit-Matth Amit-Matth commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

This PR introduces the Clip Area Tool, which works as a crop-like feature for the canvas.

PAINTROID-796

New Features and Enhancements

  • Users can define a clip area by drawing a path on the canvas.
  • On confirmation (checkmark), all content outside the clip path is cleared.
  • Only the content inside the clip path remains visible.
  • Implements behavior 1:1 with the native Paintroid app, ensuring consistent user experience.
  • Integrates with the existing command system for undo/redo support.

This provides users with an intuitive way to crop their artwork to a custom shape, matching the functionality of the native app.

Refactorings and Bug Fixes

  • Refactorings and Bug Fixes

Checklist

Your checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

  • Include the name of the Jira ticket in the PR’s title
  • Add the link to the ticket in Jira in the description of the PR
  • Include a summary of the changes plus the relevant context
  • Choose the proper base branch (develop)
  • Confirm that the changes follow the project’s coding guidelines (Wiki)
  • Verify that the changes generate no compiler or linter warnings
  • Perform a self-review of the changes
  • Verify to commit no other files than the intentionally changed ones
  • Include reasonable and readable tests verifying the added or changed behavior
  • Confirm that new and existing tests pass locally
  • Check that the commits’ message style matches the project’s guideline
  • Verify that your changes do not have any conflicts with the base branch
  • After the PR, verify that all CI checks have passed
  • Add new information to the Wiki

@Amit-Matth Amit-Matth changed the title PAINTROID-796 initial setup PAINTROID-796 Add Clip area tool Aug 14, 2025
@juliajulie95

Copy link
Copy Markdown
Contributor

@Amit-Matth Please regenerate the conflicted file

@Amit-Matth

Copy link
Copy Markdown
Contributor Author

@juliajulie95 PR is ready for review now.

@juliajulie95

Copy link
Copy Markdown
Contributor

In the native version the dashed line has a black border, so that when you still have the same color selecting you drew with, you still see exactly where you selected.
Please add this

Everything else looks fine! Thank you

@Amit-Matth

Copy link
Copy Markdown
Contributor Author

In the native version the dashed line has a black border, so that when you still have the same color selecting you drew with, you still see exactly where you selected. Please add this

Everything else looks fine! Thank you

Thanks for the suggestion! I’ve already added the black border to the dashed selection line to match the native version and pushed the changes.

@juliajulie95

Copy link
Copy Markdown
Contributor

There are new merge conflicts, due to merging your other ticket, please resolve

Comment thread lib/core/commands/command_implementation/graphic/clip_path_command.dart Outdated
Comment thread lib/core/commands/command_implementation/graphic/clip_path_command.dart Outdated

@juliajulie95 juliajulie95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small comment

@Amit-Matth
Amit-Matth requested a review from juliajulie95 May 21, 2026 11:04
Comment thread lib/core/tools/implementation/clipping_tool.dart Outdated
@juliajulie95

Copy link
Copy Markdown
Contributor

@Amit-Matth

I let AI run over the code and it found some issues, please check.
If one isnt correct, just answer why, you dont need to make these changes if they are not good by your opinion

  1. The dashed preview is a real command on the undo stack. ClipPathCommand goes through addGraphicCommand, is @JsonSerializable and is registered in Command.fromJson. If you draw an area and then switch tools or leave the page without pressing ✓ or cancelling, the dashed outline stays on the stack — RenderImageForExport runs executeAllCommands, so it gets baked into the exported PNG and the saved project. In the Android version the preview isn't a command at all (ClippingTool.draw(canvas) renders it per frame; only ClippingCommand is ever added), and leaving the tool is handled explicitly: checkForImplicitToolApplication() applies the clip on tool switch, adjustClippingToolOnBackPressed() discards it on back. We have neither hook.

  2. Pressing ✓ twice adds a second ClipAreaCommand and appends another CloseAction to the same path. Android guards this by resetting areaClosed = false and pathToDraw.rewind() at the end of onClickOnButton(). Same for pressing ✓ after a cancel — pathToDraw still holds the discarded path.

  3. onCancel throws away a finished clip area, not just the in-flight stroke. It's only called from didSwitchToZooming(), so pinch-zooming to check your selection silently deletes it. Android's mid-gesture reset never touches areaClosed.

  4. Race in onUp: resetCanvasWithExistingCommands() and updateCachedImage() are both async and neither is awaited, so they run concurrently and whichever toImage() finishes last wins. requiresRefresh is always true there (onDown always sets _liveDrawingCommand), so this happens on every stroke.

  5. commandManager.removeCommand uses List.remove, i.e. ==. Commands are Equatable and PathWithActionHistory has value equality, so it removes the first equal command, not the intended instance. Mostly moot if the preview stops being a command.

  6. ClipAreaCommand depends on an ambient saveLayer. BlendMode.clear only composites correctly because canvas_painter.dart now wraps the whole painting layer in Opacity(0.99). That also means the saveLayer is unconditional now (it used to be skipped while erasing). A saveLayer/restore inside ClipAreaCommand.call would make the command self-contained and let the Opacity go back to being conditional.

  7. Preview paint: the contour colour is hardcoded black, so a black clip stroke gives an invisible outline. Android picks the contrast colour (if (previewColor == BLACK) WHITE else BLACK) and uses strokeWidth + 5 instead of * 1.2. Android also forces strokeWidth = STROKE_10, style = STROKE and hideCaps() — here StrokeToolOptions is handed over unmodified, so stroke style/caps carry over from the previous tool. Also, dash gap is strokeWidth * 1.5 vs * 2f in Android — intentional?

  8. Dead code: DASHED_PATH_COMMAND, DASHED_PATH_COMMAND_VERSION and getDashedPathCommandVersion() (interface + prod strategy + DummyVersionStrategy) — there's no DashedPathCommand class anywhere.

Smaller structural note: Android closes the area by mutating the path once in handleUp (quadTo(coordinate, initialCoordinate)) and rejects a bare tap outright. Doing that instead of deferring the closing line to ClipPathCommand.call via startPoint/endPoint would remove the isEffectivelySingleTap branch and ~40 lines from onUp — and the two params are currently passed inverted (startPoint: currentEndPoint, endPoint: _startPoint), which is confusing.

@Amit-Matth

Copy link
Copy Markdown
Contributor Author

I've finished the Clipping Tool alignment. The main change was moving the preview logic out of the Command stack and into draw(), which prevents previews from being baked into exports. I also matched the Android version's contrast border and dash ratios, removed the dead code, and fixed the selection so it now persists correctly during zoom gestures.

Thanks for the detailed review! It was really helpful and gave me a much clearer direction on what needed to be fixed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants