After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 343358 - Eyedropping should select a matching palette entry if possible
Eyedropping should select a matching palette entry if possible
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: User Interface
git master
Other All
: Normal enhancement
: 2.4
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2006-05-30 05:24 UTC by david gowers
Modified: 2006-08-27 12:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proof of concept patch (1.51 KB, patch)
2006-06-01 04:33 UTC, david gowers
none Details | Review
updated patch (clarified, optimized, and conforms to coding standards) (3.72 KB, patch)
2006-06-06 11:57 UTC, david gowers
none Details | Review

Description david gowers 2006-05-30 05:24:13 UTC
Following on from bug #130123, this would improve palette editor usage further.
The idea is:

When eyedropping, try to find and select a color in the palette that matches the color just collected. First check if the currently selected entry in the palette matches the color eyedropped[1]; if not, search the palette for a matching color and select that. If no match is found do nothing.


This is intended to support the next/prev color in palette functionality recently added (see bug #130123 ), and improve the usefulness of eyedropping.

I've already got an idea how to implement this, and a partial prototype. I would like input as to whether this matching should occur:
 a) Only when eyedropping
 b) When eyedropping, or when FG/BG is changed via drag-n-drop 
 c) Anytime FG/BG color changes.
 d) Not at all
 e) (something else)

Speed should not be an issue, I worked out a way to search the palette much quicker than usual.
(b) is what I would pick offhand.

[1] This is to accommodate palettes where the same color occurs multiple times.
Comment 1 Sven Neumann 2006-05-30 15:09:03 UTC
IMO this only makes sense for indexed images and if I remember correctly, this is already implemented.
Comment 2 david gowers 2006-05-31 07:07:18 UTC
If you say it only makes sense for indexed images, then yes it is already implemented, otherwise not.

Who actually draws indexed images in indexed mode? Mostly it's a nuisance; rearranging the order of the colormap is painful and adding new colors is even more painful (unless you actually want to stick them on the end of the colormap.), plus drawing modes and layers are crippled.

The two reasons for this proposed change:

 * Assisting quick movement through the palette (eyedropping should position the palette editor cursor near related colors)

 * Assisting work on 'mostly-indexed' images (meaning images that are intended to be indexed and thus use certain predefined colors, but also include some not-yet-paletted bits that will be adjusted to suit indexization at a later time-- classic example is a 'index-based' image that you want to use a lighting effect or drawing mode on, temporarily putting some of the image colors outside the indexed palette defined. I even made and often use a plugin specifically to do that (indexize with preselected params then return the image to RGB mode), which should show how commonly it can be useful with indexed stuff.)


-
I now have a fully working prototype (naive and thus slow, but fully usable). I might attach it if I think it's going to take a while to implement an optimized system.
Comment 3 david gowers 2006-06-01 04:33:06 UTC
Created attachment 66572 [details] [review]
proof of concept patch

This patch turned out a lot better than I thought -- it's actually quite fast on my system (800mhz duron) for at least palettes up to 256 entries (eyedropping colors from the end of the palette to work it hardest). It implements the idea in full, but not the optimizations.

I have access to a 233mhz p2, which I will use to test this on more modestly specced hardware later.

Mainly putting this here so anyone who wants to try can play with it and see if they like the way it changes eyedropping.
Comment 4 david gowers 2006-06-06 11:57:50 UTC
Created attachment 66815 [details] [review]
updated patch (clarified, optimized, and conforms to coding standards)

Time warp..
Actually I don't have access to such a machine -- that machine was done away with a few months ago. Oops.

Anyway, doesn't appear to cause any slowdown on either of the machines I tested it on, and behaviourwise it matches my idea of what should happen (palette editor cursor keeps up with eyedropped colors if they are in the palette)
(is there anyone who can test this on more modestly-specced machines?)

This patch optimizes and clarifies the function somewhat ( only look for the dockable factory and palette editor dockable once.)

It would be easy to optimize further later if needed, without any more changes to app/tools/gimpcolortool.c.

To elaborate more on what I said before about the use of such a facility for RGB images, I've been involved in various pixel art communities; as you may know, pixel art involves limited (often severely limited, 6 to 24 ) colors; I've found despite that, a majority of the pixel artists I've talked to use RGB/24bit mode because it's just more convenient (even with good palette management, Indexed is still more complicated than RGB to manage.)
I've also found it about equally useful for non-pixelart images. Any picture  that uses flat color areas rather than ultra-smoothed/airbrushed areas could probably be improved most readily with the assistance of this function.

This patch should be ready to commit.
Comment 5 Michael Natterer 2006-08-27 12:20:06 UTC
Applied a slightly version of the last patch. Closing as FIXED.

2006-08-27  Michael Natterer  <mitch@gimp.org>

	* app/tools/gimpcolortool.c (gimp_color_tool_real_picked): applied
	modified patch from David Gowers which selects a matching color
	from the palette editor's active palette. I'm not sure if this
	behavior is desirable but we'll never find out if we dont't try.
	Fixes bug #343358.