search

lbalazscs / Pixelitor

A desktop image editor
https://pixelitor.sourceforge.io/
GNU General Public License v3.0
181 stars 70 forks source link

Magic Wand Feature #338

Closed AnthonyyHL closed 8 months ago

AnthonyyHL commented 8 months ago

Hello!

This is the PR with the Magic Wand feature. I hope it aligns with the project's goals and hope you like it!

lbalazscs commented 8 months ago

This is great code, and I'm merging this PR. Welcome aboard as a Pixelitor contributor! I must say I'm impressed, because selection handling is one of the more complex parts of Pixelitor. However, I found some problems in the PR. I hope that you will fix them in a separate PR (or in multiple PRs). If not, I'll try to fix them myself.

  1. Clicking outside the canvas while in Magic Wand throws an exception.
  2. When using one of the flat themes (see Edit/Preferences/UI/Theme), after switching from Magic Wand to another selection mode, some parts of the Tolerance slider don't disappear (it's some sort of repainting problem). In general, I think it would be better to just disable the slider (instead of removing it); it would be more consistent with how other controls work. For example, when there's no selection, the "Crop Selection" button doesn't disappear; it just becomes disabled. The long-term solution would be to split the selection tool into five separate tools. However, this is tricky because all tool buttons must be visible even on small laptop screens.
  3. It's not clear what the right-click does. The status bar message says that right-click closes the selection, but while I know what closing means for polygonal selections, it's not clear what it means for Magic Wand. Is this a copy-paste error?
  4. The biggest problem is that if I click on a large photo with a large tolerance, then Magic Wand becomes very slow. A user might simply conclude that Pixelitor is "frozen" and the process must be killed. I think the issue is that you create an Area object for every pixel, and Area operations are relatively slow. There must be some way to accumulate fewer but larger Areas before merging them.
  5. This is not a bug, but I think that the resulting shape could be post-processed with a PathIterator to be smoother. Currently the shape perfectly aligns with the pixel boundaries, but users might prefer a diagonal line to a "jagged" staircase, because Pixelitor filters perform automatic anti-aliasing for angled lines. To see what I mean, select a triangle using the Polygonal selection, and press Ctrl-I (Invert filter).

BTW, don't worry about the "project goals": as long as a feature is commonly found in other image editors, it's a project goal to also add it to Pixelitor. There's a roadmap at https://github.com/lbalazscs/Pixelitor/blob/master/stuff/Roadmap.md but that is very incomplete.

AnthonyyHL commented 8 months ago

I'm sorry for the problems. I'll try to work on fixing them gradually.

At the same time, I'm glad that the new feature had the opportunity to become part of this project :)

lbalazscs commented 8 months ago

Great! Keep in mind that you don't need to wait until you've fixed all the issues. It's OK to fix them through multiple pull requests.

In the meantime I tried to research algorithms that don't use Area, and I found this: https://losingfight.com/blog/2007/08/28/how-to-implement-a-magic-wand-tool/ It seems that most people are using this algorithm for Magic Wand. For example this seems to be an (Android) Java implementation: http://www.java2s.com/Open-Source/Android_Free_Code/App/draw/com_jaween_pixelart_toolsMagicWand_java.htm

I think this algorithm should be much faster than using Areas, because Area is a very complex and powerful class, but here we don't need all that power, and on the other hand we could deal with millions of pixels.