GNOME Bugzilla – Bug 760481
Add "diagonal neighbors" option to fuzzy-select and bucket-fill
Last modified: 2016-01-17 13:24:37 UTC
Created attachment 318767 [details] "Diagonal neighbors" demonstration for fuzzy-select The individual pixels of thin lines and curves, especially of non antialiased ones, are often connected to each other only by diagonal adjacency. However, the fuzzy-select and bucket-fill tools can only calculate their affected region based on orthogonally adjacent pixels, which can make selecting, and filling, thin curves tricky. The following patchset adds a "diagonal neighbors" option to both of these tools. When this option is checked, the tools consider diagonally neighboring pixels, as well as orthogonally neighboring ones, when calculating the affected region. In other words, instead of only looking at the four orthogonal neighbors of each pixel, they look at all eight neighbors. The attached screenshot demonstrates the effect of this option when using fuzzy-select to select a thin curve.
Created attachment 318769 [details] [review] 0001-app-Add-diagonal_neighbors-parameter-to.patch
Created attachment 318770 [details] [review] 0002-app-Add-Diagonal-neighbors-option-to-the-fuzzy-selec.patch
Created attachment 318771 [details] [review] 0003-app-Add-Diagonal-neighbors-option-to-the-paint-bucke.patch
Created attachment 318772 [details] [review] 0004-app-Add-diagonal_neighbors-parameter-to-gimp_channel.patch
Created attachment 318773 [details] [review] 0005-pdb-Add-diagonal-neighbors-as-GimpPDBContext-propert.patch
Created attachment 318774 [details] [review] 0006-pdb-Use-the-diagonal-neighbors-setting-from-GimpPDBC.patch
A few notes about the patches: The entire "logic" is contained in the 5 new lines in find_contiguous_region(), in the first patch; the rest is boilerplate/UI/PDB. Patches 2 and 3 add the option to the fuzzy-select and bucket-fill tools, respectively. Patches 4-6 add the relevant PDB changes.
Created attachment 318810 [details] [review] 0001-app-Add-diagonal_neighbors-parameter-to.patch Avoid a theoretical integer overflow possibility in the previous version of the patch. Be noted that patch 0001 will probably appear LAST now---it's still the first patch.
Holy shit, this is a totally complete patch set with everything done right, that almost never happens :) I think this feature makes a lot of sense, I'll bring it up on IRC.
My feeling is that this should be a toggleable option with a modifier key. I can't tell whether it is or not by looking at the patches.
(In reply to Michael Schumacher from comment #10) > My feeling is that this should be a toggleable option with a modifier key. I > can't tell whether it is or not by looking at the patches. No modifier key ATM, but I agree it would be useful. Taking a quick look at the keyboard shortcuts list, though, there doesn't seem to be modifier keys for any of the boolean tool options (and shift/ctrl/alt are all taken for selection), so that would be a first?
Created attachment 318900 [details] [review] 0007-app-Add-a-modifier-key-to-toggle-the-diagonal-neighb.patch The attached patch adds a modifier key to toggle the diagonal neighbors option. AFAICT that's the only toggleable tool option with a programmable modifier key, so you might want to take a closer look at this.
That patch is nice and correct too, but not what was meant :) Michael really meant a modifier key, and we indeed have none left, so I guess we can safely ditch that idea. IMO this feature is pretty special anyway (if not scientific / totally nerdish), so let's just go with that toggle without any modifier. As to the feature itself, I think we can justify pushing these patches, they seem to properly implement an 8-neighborhood by the book.
(In reply to Michael Natterer from comment #13) > That patch is nice and correct too, but not what was meant :) > > Michael really meant a modifier key, and we indeed have none left, so I guess > we can safely ditch that idea. IMO this feature is pretty special anyway > (if not scientific / totally nerdish), so let's just go with that toggle > without any modifier. > > As to the feature itself, I think we can justify pushing these patches, > they seem to properly implement an 8-neighborhood by the book. Didn't realize there were two Michaels :) FWIW, I'm completely fine with whatever: it is handy not to have to muck around with the mouse while doing a selection, and on the other hand it's completely awkward to have this as the only option with a key binding.
Created attachment 319049 [details] [review] 0003-app-Add-Diagonal-neighbors-option-to-the-bucket-fill.patch s/paint bucket/bucket fill/ in commit message.
Thanks again for the patches! Fixed in master: commit 7b89fe65366e37934be580756f68eefad92d7453 Author: Ell <ell_se@yahoo.com> Date: Mon Jan 11 10:44:21 2016 +0000 pdb: Use the diagonal-neighbors setting from GimpPDBContext... ...in gimp-image-select-contiguous-color() This and the last 5 commits fix bug 760481. commit 7b3e5ba1e94f078521ca362ae8133f47e2470d8e Author: Ell <ell_se@yahoo.com> Date: Mon Jan 11 10:41:23 2016 +0000 pdb: Add "diagonal-neighbors" as GimpPDBContext property and add PDB API to get/set it. commit e0b1aa1c26660f99f65b72081ec0167d765eacb5 Author: Ell <ell_se@yahoo.com> Date: Mon Jan 11 11:02:27 2016 +0000 app: Add diagonal_neighbors parameter to gimp_channel_select_fuzzy() and propagate it to gimp_pickable_contiguous_region_by_seed(), in preperation for adding a diagonal-neighbors setting to PDB. commit 350c7ca338dcb37e3df6e13fb4dc603489aca166 Author: Ell <ell_se@yahoo.com> Date: Fri Jan 8 13:18:10 2016 +0000 app: Add "Diagonal neighbors" option to the bucket fill tool When checked, diagonally neighboring pixels are considered connected when calculating the affected area. This commit also adds a corresponding diagonal_neighbors parameter to gimp_drawable_bucket_fill(), and modifies the callers, other than the bucket fill tool, to pass FALSE for this parameter, to retain the current behavior. commit 070007d8914df8c586e679cb10726197b862d884 Author: Ell <ell_se@yahoo.com> Date: Fri Jan 8 13:09:45 2016 +0000 app: Add "Diagonal neighbors" option to the fuzzy select tool When checked, diagonally neighboring pixels are considered connected when calculating the affected area. commit 93bf78b83e4db6add88c79f1c1516c0aa1c881ea Author: Ell <ell_se@yahoo.com> Date: Fri Jan 8 12:53:07 2016 +0000 app: Add diagonal_neighbors parameter to... ...gimp_pickable_contiguous_region_by_seed(), in preperation for adding a similar option to the relevant tools. When this parameter is TRUE, all eight neighbors of each pixel are considered when calculating the resulting region, instead of just the four orthogonal ones. This commit also modifies all callers to pass FALSE for this parameter, to retain the current behavior.
(In reply to Michael Natterer from comment #16) > Thanks again for the patches! Fixed in master: Thanks! A small heads up: I already added gimp_context_{get,set}_diagonal_neighbors() to gimp.def in one of the patches, albeit not in alphabetical order, and your commit accidentally adds the set() variant twice, so there are some duplicates now.
Argh, thanks, will fix this right away, it was late when I did this ;)
Also, I can hardly remember when I last pushed patches without changing a line. Those patches were great, can we keep you? :) Would you like to join us on IRC? You obviously know what you are doing, and we need contributors!
For the record: commit 80faa72617632140c8fb6ccca35a7d21cec33c66 Author: Michael Natterer <mitch@gimp.org> Date: Sun Jan 17 14:22:35 2016 +0100 libgimp: remove duplicate entries from gimp.def libgimp/gimp.def | 2 -- 1 file changed, 2 deletions(-)