GNOME Bugzilla – Bug 141927
new HSV gradient coloring types: shortest and longest path
Last modified: 2018-05-24 11:05:18 UTC
the current HSV blending means, getting a rainbowy transition or a simple transition is dependent on the actual colors involved. this is not very useful to me, i usually want to gradient along shortest path for the simplest blend. i have implemented a find_shortest_path function that returns CW or CCW depending on which is the shortest path to take through the hue wheel; i have tested this with the fg->bg gradients and it works well. i would like to implement two new HSV blending modes, one which travels along the shortest path along hue, and one which travels along the longest path. i want to make these available in the menus and obsolete the old ones, since these provide the old functionality in a more consistent way.
here is a patch (attached)
Created attachment 27448 [details] [review] implements shortest/longest path coloring types for gradients. this patch expands the gradient editor coloring type submenu to a total of 5 entries; i would like to eventually get rid of CW and CCW from the interface.
Should be considered for 2.2.
There's one point in the patch that I don't understand: /* because CW and CCW are currently reversed, must reverse direction */ What is reversed and why? As it stands the comment seems to be more confusing than helpful.
I have modified it to be more understandable. CW and CCW are reversed; requesting a CW gradient results in a CCW one and vice versa. I've verified that this is not just my code, by testing it outside of GIMP. I tried to fix this problem but couldn't figure out the origin of it. patch attached with updated comments and using the new GIMP_GRADIENT_SEGMENT_* names. may modify some autogenerated files: i wasn't sure which were autogenerated so i left those parts in.
Created attachment 28389 [details] [review] updated patch
The GradientSegment API is new in the PDB so there is no point in keeping the old enum values. If we decide to accept this change then CW and CCW should be removed.
excellent. however, the old CCW and CW will now become equivalent to shortest and longest path when loading old gradients. i see no way to avoid this except padding the enum with dummy entries. attaching an updated patch removing CW/CCW
Created attachment 28406 [details] [review] newest patch (CW/CCW is removed)
You misunderstood me. Of course we cannot completely remove the old enum values since it would break the gradient format. What I meant was to remove the CW and CCW values from the exported API. The PDB generation code allows enum values to be skipped so we wouldn't have to export the deprecated values to PDB (and libgimp).
i'm not 100% sure if this is correct: + i left cw/ccw menu entries in, because it's confusing when none of the menu entries in the coloring type menu are selected. + gimpenums.h still has CCW and CW. i assume this is necessary to make the values used match with core-enums.h. i'm pretty sure python,perl,and script-fu constants are correct. updated patch attached.
Created attachment 28457 [details] [review] latest patch (CW/CCW removed from PDB)
is this likely to make it into 2.2 or not? is there anything further i can do to improve the likelihood of the patch being accepted?
It's on the milestone, it has the PATCH keyword set. So it will be considered at least.
Created attachment 33228 [details] [review] updated patch This is an updated version of the patch that applies cleanly against current CVS. It has CW and CCW added back to the enum since skipping enum values in the middle of an enum isn't really such a good idea. It also fixes (hopefully all) coding style issues with the patch. I am not sure if I am happy with this change for 2.2. Any comments anyone?
Bumping to the Future milestone since there hasn't been any feedback here and we need to draw a line now and declare the GIMP 2.2 API completely frozen.
Is this now good to commit?
It probably would be good to commit if we decided that we want to do this change.
I'm in favor of replacing CW and CCW HSV gradients with "simple" and "complex" ones (better names than "long" and "short" imho). I think it would be easier for users to make sense of. An added benefit is that bug #141922 could then be WONTFIX'ed.
Is this something that I can do?
If you want to do this you should use the following patch instead of Sven's last one; It is updated to apply to CVS HEAD (and longest/shortest path has been changed to simple/complex).
Created attachment 58191 [details] [review] updated patch applicable to CVS HEAD, inc change 'longest/shortest' -> 'simple/complex'
(Setting patch status to Reviewed based on comment #18)
Created attachment 120425 [details] [review] updated patch, applies to SVN HEAD as of Oct 12 2008 I think this would be a good and non-controversial improvement to make for 2.7. Does Peter need to have input here? Otherwise, the only thing I know that might need reviewing is whether we need to mark CW and CCW as deprecated in the menus.
Frankly, I don't understand why you think that the terms CW/CCW and shortest/lonest path are considered mutually exclusive. To me they rather represent two ways of saying the same. "Simple" and "Complex" mean nothing to me while clockwise and counter-clockwise make total sense. What is the "simple" way for two points on the hue circle that are almost exactly opposite? CW and CCW seems to make more sense in such a case. Also, if you selected "shortest/longest path" and change one of these almost opposite color hues, the path would suddenly flip to the other side. That feels totally broken to me. This whole bug seems to me as an attempt to change the enum values to sometihng that is not clearly better, but rather your presonal preference.
An example of why this is clearly better: Try the patch, edit a gradient, choose 'hsv CW' or 'hsv CCW' for a segment. Now flip the segment -- the appearance of the segment became something entirely different! This does not occur with shortest / longest path, and in my experience it's really a confusing behaviour. It was actually that problem that led me to develop this patch in the first place. (which suggests that perhaps we could invert CW/CCW upon flipping, instead. Is that more or less confusing than the current misbehaviour?) The meaning of simple/complex relate to the amount of color 'bands' in the path taken. the entire hue spectrum comprises 6 bands, and any given transition can either take the path which has least bands (simple) or most bands (complex). Does the visual correlation of these terms not work for you?
I'm sorry, but it doesn't work at all for the example I gave with start and end point at opposite sides of the circle. It also doesn't work when changing a segment endpoint's hue, because that will make the colors flip to the other side when you cross 180°. I fully see the problem with "flip" when it comes to CW/CCW, but maybe we should rather fix that instead of changing it to something that has different problems.
Actually, I want a consistent behavior, and prefer to keep CW & CCW whether they are fixed or not for direction. If you want to add additional HSV gradient behaviors such as shortest/longest distance, that is fine, but please don't take CW & CCW away just because you personally wouldn't use it.
To clarify, if I am going between approximately purple and approximately green (approximately on opposite sides of the color wheel) then, assuming CW and CCW refer to the same direction, I know purple CW to green will always be via yellow and purple CCW to green will always be via blue. I don't have to worry whether I am going +179 degrees or -179 degrees. With shortest/longest path, I would have to worry about that. purple shortest path to green +179 degrees away would be via blue and -179 degrees away would be via yellow (or vice versa).
Sorry, "assuming CW and CCW refer to the same direction" should read "assuming CW and CCW refer to the proper direction".
After reading the different comments, I'd suggest the following: 1. Don't mark CW and CCW as deprecated, either in the code or in the menus. I feel the terms have meaning to graphic artists and are especially helpful when dealing with gradients between colors nearly opposite one another on the color wheel. I rather image that graphic artists have a clear image of the color wheel 1a. As a side issue, I believe it would be confusing to end users to mark *any* menu item as "deprecated." 2. I don't necessarily agree that CW and CCW are swapped in GIMP (red to blue CW goes via purple where green is expected; red to blue CCW goes via green where purple is expected). My perception actually agrees with the person who filed this bug, that clockwise proceeds red, green, blue. Also, in terms of hue, red=0 or 360 degrees, green=120 degrees and blue=240 degrees. If we assume that degree numbering proceeds as clock numbering does, then GIMP does swap CW and CCW in gradients processing. But does it? Where in GIMP is there an actual color wheel which shows that degree increase in a clockwise direction. I just searched for "rgb color wheel" (without the quotes) in Google images. The numbers for a clockwise red, green, blue are approximately the same as a clockwise red, blue, green. Notably the one on the Adobe Web site is clockwise red, green, blue. But then, GIMP is not Photoshop. The bottom line is there appears to be no consensus as to which direction red to green to blue goes, so GIMP is not "wrong." Perhaps there needs to be a separate preference setting as to which direction color wheel the end user has as their mental model. Then GIMP would follow that model. I have logged this as a proposed enhancement, Bug #577167. 3. Looking at the proposed patch file I'm concerned about the following code attempting to "fix" color wheel directionality: + /* To get CW coloring, must request CCW and vice versa: + * GIMP treats CW as CCW and CCW as CW: i haven't found why. + */ + if (colortype == GIMP_GRADIENT_SEGMENT_HSV_CCW) + colortype = GIMP_GRADIENT_SEGMENT_HSV_CW; + else + colortype = GIMP_GRADIENT_SEGMENT_HSV_CCW; My concerns are a) this may lead to problems elsewhere, b) the same hack might have to be duplicated wherever there is HSV gradient processing, and c) not everyone may agree than GIMP has it backwards. 3. I'd suggest "Fewer Bands" and "More Bands" in place of "Simple" and "Complex" so that end users don't have to map the meaning of "Simple" and "Complex" onto the result.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gimp/issues/77.