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 128367 - Use of (changeable) forgeground/background colors in gradients
Use of (changeable) forgeground/background colors in gradients
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: General
1.x
Other All
: Normal enhancement
: 2.4
Assigned To: GIMP Bugs
GIMP Bugs
: 333822 (view as bug list)
Depends on:
Blocks: 127676
 
 
Reported: 2003-12-02 15:21 UTC by TechChiq
Modified: 2006-08-31 23:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch showing changes to gimp_gradient_get_color_at (6.30 KB, patch)
2006-01-31 14:50 UTC, david gowers
none Details | Review

Description TechChiq 2003-12-02 15:21:08 UTC
At the request of Sven in <a
href="http://bugzilla.gnome.org/show_bug.cgi?id=128353">Bug #128353
</a> I'm resubmitting this as an Enhancement Request.

It would be very helpful to be able to use the current foreground and/or
background in a gradient, so that if the user changes the foregrounds
and/or background, the gradient compensates. Just like the (uneditable)
FG/BG gradients currently in Gimp. Only it would be nice to be able to
create your own.

For example, here's how a user would construct such a gradient:

1. Start to create a new gradient.

2. Right Click and change the Foreground or background color:
   Either click on Load Left Color From or Load Right Color From
   Then choose either "From FG" or "From BG" (these could be added as
   menu options)

3. Save the gradient

4. Change the foreground and/or background color (depending on what
   you changed in Step 2. Ie. if you have it load from FG then change
   the foreground color in Gimp's toolbox).

The gradient could then pick up the foreground and/or background color and
apply the gradient with the currently selected foreground and/or background
inserted in the gradient.

As for the file format, maybe such a "color" would be designated as
"-1.000" (ie. as a negative number) to indicate to GIMP that it's using a
foreground color and "-2.000" to indicate it's to use a background color.

I have a few Paint Shop Pro 8 gradients that I wish to convert to Gimp that
rely on foreground and/or background colors to be inserted into the
gradient and changed.
Comment 1 Sven Neumann 2003-12-02 15:37:11 UTC
This would be nice to have and it should be considered for
implementation. It would also open a nice way to solve bug #127676.
Comment 2 david gowers 2006-01-28 08:52:56 UTC
This idea has usability problems in it's present form;
It makes 'blend endpoints' and 'split segment' nonsensical, unless you treat the embedded-FGcolor as a normal color when blending-- in which case you get undesired effects when your FGcolor changes.
Prohibiting splits and blends on ranges containing embedded FG/BG colors is a confusing and inappropriate behaviour.

If that conceptual mistake can be fixed, I may implement this.
If not, I suggest resolving this WONTFIX.
Comment 3 Michael Schumacher 2006-01-30 23:27:25 UTC
If we'll have special "fg" and "bg" points, can't we have "computed from fg/bg blending" points as well?
Comment 4 david gowers 2006-01-31 05:16:14 UTC
That doesn't change the basic UI problem:
You can setup a set of endpoints such that some endpoints are '50% FG, 50% next endpoint on left'. Via 'blend endpoints'.
But how would you SET such a color individually on an endpoint?
Comment 5 Michael Schumacher 2006-01-31 12:00:32 UTC
I fail to see the conceptual mistake in this, thzough - there will be a new feature, and we now have to come up with a suitable UI for it. The color selection dialog could be changed, for example.

But you seem to have an idea how this could be implemented for the "pure" fg and bg points, maybe you could explain how you want to make it obvious to the user that these points follow the current fg/bg colors? I fail to see how this is different from telling them that a point is composed from "50% fg and color of next point (bg or real color)", for example.



Comment 6 david gowers 2006-01-31 13:04:03 UTC
In the normal display,
Different segment handle colors, and an informational text in the status area.
A status display like '50% [A] + 50% [B]' (where A and B are rectangles of color) would be ideal (with an arrow showing the source of any non-FG/BG color, either from next endpoint on left or on right), though that might be difficult to make coexist with the current labels used for messaging.

.. I thought I knew how to denote it in the menus, but found I was wrong.

Also, if such percentage mixes are allowed, how does get_color_at gain access to the Gimp object needed to get the current FG/BG?
My current backend cheats in this regard; it expects negative values that equate to acceptable RGB when put back in range.
Comment 7 Michael Schumacher 2006-01-31 13:41:44 UTC
Maybe you should attach your changes here, so that we can discuss this while looking at the code?
Comment 8 david gowers 2006-01-31 14:50:26 UTC
Created attachment 58469 [details] [review]
patch showing changes to gimp_gradient_get_color_at

this part:

+  /* suggested method for handling FG/BG: 
+  
+     embedded FG = RGBA -1.0 (values ranging -1.0 to -2.0)
+     embedded BG = RGBA -4.0 (values ranging -4.0 to -5.0)
+     something else (?) RGBA -8.0 (values ranging -8.0 to -9.0)

summarizes it.

The patch is mixed up with the other feature I neatened up recently -- HSV simple/complex coloring types. It should be easy to distinguish the two.
Comment 9 Michael Schumacher 2006-02-01 21:32:54 UTC
So you didn't think about the UI for this yet, correct?
Comment 10 david gowers 2006-02-02 06:19:32 UTC
Correct. Ideas follow.


The color selector could have a mode switch between direct and derivative color selection. (checkbox or radiobutton)
If it did that I would want it to make sense in other contexts too.
For instance, being able to select palette colors as a mixture of other colors in that palette; (and the same for colormaps). That would also be very useful for fg/bg (colors derived from entries in current colormap or palette), if we could change the mixing ratio from the keyboard!

Possible layout for derivative mode:


--- 

A: [Kindmenu]
Selector (for colormap and palette only)

B: [Kindmenu]
Selector (for colormap and palette only)

Location: [spinbutton]

---


[Kindmenu] 
would include
FG
BG
Colormap entry
Palette entry
Current gradient 

for FG/BG selection FG/BG would be disabled.
for gradients Current gradient would be disabled.
for 'Current gradient' the data relating to B would be disabled and the Location spinbutton would indicate location in the gradient.

Selector:
A compact interface to colormaps and palettes.
Would show one line of the palette or colormap colors, with
a way to step through it (cursor keys and scrollwheel at least, probably also scrollbuttons that scroll by 1 when clicked and by (palette->columns, or 16 in the case of colormaps) when shift clicked)
Comment 11 Joao S. O. Bueno 2006-03-09 05:50:24 UTC
*** Bug 333822 has been marked as a duplicate of this bug. ***
Comment 12 Michael Natterer 2006-08-29 21:50:29 UTC
The internal part is there. Loading/Saving and GUI still missing...

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

	Changed GimpViewable preview rendering to have a context to get
	FG/BG/whatever from. Use the context to enable dynamic FG/BG
	colors in gradients. Fixes bug #127676 and bug #352214. Addresses
	bug #128367 (doesn't fix it because there's no loading/saving and
	no GUI yet).

	* app/core/core-enums.[ch]: added enum GimpGradientColor to enable
	specifying gradient colors in terms of foreground and background.

	* app/core/gimpgradient.[ch]: added color_type members to the
	GimpGradientSegment struct and honor them in
	gimp_gradient_get_color_at(). Added GimpContext parameters to all
	functions which finally call get_color_at().

	* app/core/gimp-gradients.c: use the new method to implement the
	builtin gradients.

	* app/core/gimpviewable.[ch]: added GimpContext parameters to all
	get_preview() and get_pixbuf() functions.

	* app/core/gimpbrush.c
	* app/core/gimpbuffer.c
	* app/core/gimpdrawable-preview.[ch]
	* app/core/gimpgradient.c
	* app/core/gimpimage-preview.[ch]
	* app/core/gimpimagefile.c
	* app/core/gimppalette.c
	* app/core/gimppattern.c
	* app/core/gimpundo.[ch]
	* app/text/gimpfont.c
	* app/vectors/gimpvectors-preview.[ch]: changed ::get_preview()
	and ::get_pixbuf() implementations accordingly.

	* app/core/gimpdrawable-blend.c
	* app/core/gimppalette-import.[ch]
	* app/dialogs/dialogs-constructors.c
	* app/dialogs/palette-import-dialog.c
	* app/dialogs/resize-dialog.c
	* app/display/gimpdisplayshell-layer-select.c
	* app/display/gimpdisplayshell.c
	* app/display/gimpnavigationeditor.c
	* app/paint/gimppaintoptions.c
	* app/tools/gimpeditselectiontool.c
	* app/tools/gimptexttool.c
	* app/actions/gradient-editor-commands.c
	* app/widgets/gimpaction.c
	* app/widgets/gimpbrusheditor.[ch]
	* app/widgets/gimpbufferview.c
	* app/widgets/gimpcellrendererviewable.c
	* app/widgets/gimpchanneltreeview.c
	* app/widgets/gimpclipboard.c
	* app/widgets/gimpcoloreditor.c
	* app/widgets/gimpcomponenteditor.c
	* app/widgets/gimpcontainerbox.c
	* app/widgets/gimpcontainercombobox.c
	* app/widgets/gimpcontainereditor.c
	* app/widgets/gimpcontainerentry.c
	* app/widgets/gimpcontainergridview.c
	* app/widgets/gimpcontainertreeview.[ch]
	* app/widgets/gimpdataeditor.[ch]
	* app/widgets/gimpdevicestatus.c
	* app/widgets/gimpdnd.[ch]
	* app/widgets/gimpdrawabletreeview.c
	* app/widgets/gimpfiledialog.c
	* app/widgets/gimpgradienteditor.[ch]
	* app/widgets/gimpgradientselect.c
	* app/widgets/gimpitemtreeview.c
	* app/widgets/gimplayertreeview.c
	* app/widgets/gimppaletteeditor.[ch]
	* app/widgets/gimppropwidgets.[ch]
	* app/widgets/gimpselectioneditor.c
	* app/widgets/gimpthumbbox.[ch]
	* app/widgets/gimptoolbox-image-area.c
	* app/widgets/gimptoolbox-indicator-area.c
	* app/widgets/gimptooloptionseditor.c
	* app/widgets/gimpundoeditor.c
	* app/widgets/gimpvectorstreeview.c
	* app/widgets/gimpview-popup.[ch]
	* app/widgets/gimpview.[ch]
	* app/widgets/gimpviewablebutton.c
	* app/widgets/gimpviewabledialog.c
	* app/widgets/gimpviewrenderer.[ch]
	* app/widgets/gimpviewrenderer-frame.c
	* app/widgets/gimpviewrendererbrush.c
	* app/widgets/gimpviewrendererbuffer.c
	* app/widgets/gimpviewrendererdrawable.c
	* app/widgets/gimpviewrenderergradient.c
	* app/widgets/gimpviewrendererimage.c
	* tools/pdbgen/pdb/drawable.pdb
	* tools/pdbgen/pdb/gradient.pdb
	* tools/pdbgen/pdb/gradients.pdb
	* tools/pdbgen/pdb/image.pdb: added tons of GimpContext members
	and parameters, implement GimpDocked::set_context() in many
	widgets. Pass these locally remembered contexts to GimpViewable
	functions. Did some minor cleanups on the way. There are still
	some minor FIXMEs around where the code uses a NULL context (which
	is allowed by the APIs)

	* app/pdb/drawable_cmds.c
	* app/pdb/gradient_cmds.c
	* app/pdb/gradients_cmds.c
	* app/pdb/image_cmds.c: regenerated.
Comment 13 david gowers 2006-08-30 01:21:18 UTC
I don't know if you were including this in 'UI': Duplicating one of the FG/BG gradients only updates after a refresh. I guess the other thing, that clicking on the gradient returns a greyscale rather than the real color, is definitely a UI issue.
Comment 14 david gowers 2006-08-31 08:19:03 UTC
Actually, some portion of the backend seems broken: 'replicate segment','subdivide segment','flip segment' need to preserve the color_type. I'll investigate if I get the time.
Comment 15 Michael Natterer 2006-08-31 09:02:17 UTC
I know, and there is much more broken stuff. I have lots of modified
files on my disk with fixes for the two problems you found, and
for more. They will be in CVS before the weekend.
Comment 16 Michael Natterer 2006-08-31 18:47:31 UTC
Getting closer (more stuff to come)

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

	* app/core/gimpgradient.[ch] (gimp_gradient_has_fg_bg_segments):
	new funtion which returns TRUE if any of the gradient's segments
	refer to FG of BG.

	(gimp_gradient_segment_get_left_color_type)
	(gimp_gradient_segment_set_left_color_type)
	(gimp_gradient_segment_get_right_color_type)
	(gimp_gradient_segment_set_right_color_type): new accessors for
	the new GimpGradientColor stuff.

	(gimp_gradient_segment_split_midpoint)
	(gimp_gradient_segment_range_flip)
	(gimp_gradient_segment_range_replicate): split, flip and replicate
	the segments' color_types too.

	* app/widgets/gimpviewrenderer.[ch]: added virtual functions
	::set_context() and ::invalidate() and call them.

	* app/widgets/gimpviewrenderergradient.[ch]: implement the virtual
	functions. Connect to the context's "foreground-changed" and
	"background-changed" signals if the gradient contains FG or BG
	colors and invalidate the renderer whenever they change.

	* app/core/gimp-gradients.c: removed signal connections which
	invalidated the gradients on FG/BG changes of the user context.
Comment 17 Michael Natterer 2006-08-31 18:57:05 UTC
This one makes the gradient editor work as expected:

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

	* app/widgets/gimpgradienteditor.[ch] (struct GimpGradientEditor):
	removed GimpContext member I added before deciding it needs to be
	added to GimpDataEditor.

	Use GimpDataEditor's context instead of the bogus one. Also use
	the data editor's context instead of the user context wherever it
	was used.
	
	* app/widgets/gimppaletteeditor.c: use GimpDataEditor's context
	instead of the user context here too.
Comment 18 Michael Natterer 2006-08-31 22:29:44 UTC
Implemented some GUI for this:

2006-09-01  Michael Natterer  <mitch@gimp.org>

	* app/actions/gradient-editor-actions.c
	* app/actions/gradient-editor-commands.[ch]: added actions and
	callbacks to select a gradient segment's left and right color
	type. Handle FG/BG color correctly in a few places. Use
	GimpDataEditor's context instead of the user context.

	* menus/gradient-editor-menu.xml: added the actions to the menu.

	Cleanup:

	* app/actions/palette-editor-actions.c
	* app/actions/palette-editor-commands.c: use GimpDataEditor's
	context instead of the user context.

	* app/actions/brush-editor-actions.c: remove unused context
	variable.
Comment 19 Michael Natterer 2006-08-31 23:58:41 UTC
Let's call this FIXED now, please open new bugs for bugs in the
implementation or missing stuff.

2006-09-01  Michael Natterer  <mitch@gimp.org>

	Extended the GIMP Gradient file format to contain the endpoint
	color types for each segment (this is backward compatible because
	old parsers just ignore excess fields at the end of segment
	lines). Fixes bug #128367.

	* app/core/gimpgradient-load.c: optionally load two more fields
	per segment line which contain the color types.

	* app/core/gimpgradient-save.c: save the color types at the end of
	the segment lines.