Issue 68 in gpick: Respect color naming preference where applicable

0 views
Skip to first unread message

gp...@googlecode.com

unread,
Feb 18, 2012, 8:07:12 AM2/18/12
to gp...@googlegroups.com
Status: Accepted
Owner: 00a...@gmail.com
Labels: Type-Defect Priority-Medium

New issue 68 by 00a...@gmail.com: Respect color naming preference where
applicable
http://code.google.com/p/gpick/issues/detail?id=68

Attached is a patch which implements support for this in the Mix Colors
dialog.

The exact scope that this preference covers is not yet clear to me, however
I can see that at least the Generate Scheme tool should also support this.


Attachments:
mixtool_naming.diff 2.0 KB

gp...@googlecode.com

unread,
Feb 20, 2012, 1:01:36 PM2/20/12
to gp...@googlegroups.com

Comment #1 on issue 68 by thez...@gmail.com: Respect color naming

This setting should affect Generate Scheme, Layout Preview and probably
other tools. Most of the tools from Secondary View menu are currently using
either specialized names or color names. Should I take care of this?

gp...@googlecode.com

unread,
Feb 20, 2012, 7:38:23 PM2/20/12
to gp...@googlegroups.com

Comment #2 on issue 68 by 00a...@gmail.com: Respect color naming preference
where applicable
http://code.google.com/p/gpick/issues/detail?id=68

Certainly some of these I have no idea what 'tool-specific' name might be.
I'll have a go at Generate Scheme and Layout Preview.

gp...@googlecode.com

unread,
Feb 20, 2012, 10:35:26 PM2/20/12
to gp...@googlegroups.com

Comment #3 on issue 68 by 00a...@gmail.com: Respect color naming preference
where applicable
http://code.google.com/p/gpick/issues/detail?id=68

In particular, the 'variations', 'brightness/darkness',and 'color mixer'
tools seem to be implemented in a way where the color calculation is
isolated from the actual naming; because of this I currently haven't
figured out how to implement tool-specific naming for them.

gp...@googlecode.com

unread,
Feb 20, 2012, 11:44:02 PM2/20/12
to gp...@googlegroups.com

Comment #4 on issue 68 by 00a...@gmail.com: Respect color naming preference
where applicable
http://code.google.com/p/gpick/issues/detail?id=68

Attached is a patch which implements support in BlendColors, LayoutPreview,
DialogGenerate, and DialogMix. It includes some work on making the Scheme
Generation tab/secondary-tool conform, which is not effective yet.

In a process, I found a bug -- secondary tools don't update when the global
state does. This means you have to trigger a call to calc() -- eg. by
dropping a color or otherwise changing a parameter, when you change
the "tool color naming" preference, before it will take effect.

Attachments:
partial_toolcolornaming_pref_support.diff 10.9 KB

gp...@googlecode.com

unread,
Feb 21, 2012, 3:25:09 AM2/21/12
to gp...@googlegroups.com

Comment #5 on issue 68 by 00a...@gmail.com: Respect color naming preference
where applicable
http://code.google.com/p/gpick/issues/detail?id=68

Scheme Generation now respects the preference.

In general for tool-specific naming, I've made the choice of "$COLORNAME
$OPERATIONNAME $OPERATIONPARAM".. for example 'Sunset Analogous 3'

Drag and Drop doesn't respect the preference yet, in the cases of Scheme
Generation and Scheme Preview.


Attachments:
partial_toolcolornaming_pref_support-schemes_fixed.diff 13.2 KB

gp...@googlecode.com

unread,
Feb 21, 2012, 3:04:00 PM2/21/12
to gp...@googlegroups.com

Comment #6 on issue 68 by thez...@gmail.com: Respect color naming

Preference is now respected when drag&dropping in Scheme Generation and
Layout Preview. Double-clicking on colors in Scheme Generation tool doesn't
use the setting.

Attachments:
partial_toolcolornaming_pref_support-dragdrop_fixed.diff 16.2 KB

gp...@googlecode.com

unread,
Feb 21, 2012, 3:10:14 PM2/21/12
to gp...@googlegroups.com

Comment #7 on issue 68 by thez...@gmail.com: Respect color naming

Attached patch shows three name assignment places in Brightness/Darkness
tool code. These are the places where preference dependent naming should be
used. Other tools should have similar functions.

Attachments:
brightness_darkness_naming.diff 1.0 KB

gp...@googlecode.com

unread,
Feb 21, 2012, 5:15:45 PM2/21/12
to gp...@googlegroups.com

Comment #8 on issue 68 by 00a...@gmail.com: Respect color naming preference
where applicable
http://code.google.com/p/gpick/issues/detail?id=68

Thanks :)
I actually spotted where to do the naming easily, for all the tools I
haven't implemented this for yet. The problem was finding out the relevant
info needed to implement the tool-specific naming. I suspect I can look
this up from the Style or through the layout_preview API.

One interesting thing I noticed earlier was that I -didn't- implement the
naming scheme in BlendColors.cpp:source_get_color(), but all methods of
transferring colors to palette including DnD seem to conform correctly.

gp...@googlecode.com

unread,
Feb 22, 2012, 11:44:11 PM2/22/12
to gp...@googlegroups.com

Comment #9 on issue 68 by 00a...@gmail.com: Respect color naming preference
where applicable
http://code.google.com/p/gpick/issues/detail?id=68

Oh, and thanks for the example of how to access the layout. I actually
missed that before, and the patch it was contained in :) It helps a lot.

gp...@googlecode.com

unread,
Feb 23, 2012, 2:09:33 AM2/23/12
to gp...@googlegroups.com

Comment #10 on issue 68 by thez...@gmail.com: Respect color naming

'Blend Colors' uses a palette preview widget, which has all
the 'ColorObject's with names defined at insertion time. This means that
the name defined in the 'calc' function is used. I attached a little patch
which makes 'Blend Colors' update results on activation, because the naming
preference change is not respected until results are 'updated'.

Attachments:
blend_colors_update_naming_preference.diff 276 bytes

gp...@googlecode.com

unread,
Feb 26, 2012, 7:00:43 AM2/26/12
to gp...@googlegroups.com

Comment #11 on issue 68 by 00a...@gmail.com: Respect color naming

This is nearing completion.

Remaining work:

* Color Mixer still needs to respect the preference.
* For Tool-Specific naming, rather than setting the name to
`style->ident_name`,
I'd like to set it to $BASENAME $IDENT_NAME where BASENAME is the
color_names_get() name of the 'main' style item. If I don't find a better
way to do this (I was hoping for a way to 'find style by name'.) by the
time I finish the adjustments to Color Mixer, I'll use plain iteration to
find the base color.


Attachments:
tcnaming-rev6.diff 26.9 KB

gp...@googlecode.com

unread,
Feb 26, 2012, 7:13:44 AM2/26/12
to gp...@googlegroups.com

Comment #12 on issue 68 by 00a...@gmail.com: Respect color naming

* and refactor to be much less copy-paste-ish

gp...@googlecode.com

unread,
Mar 2, 2012, 3:34:26 PM3/2/12
to gp...@googlegroups.com

Comment #13 on issue 68 by thez...@gmail.com: Respect color naming

I added a little helper class 'ToolColorNameAssigner' (revision
841090ddb4ca) to help avoid copy-pasting the same code multiple times.
Usage examples are in files 'BlendColors.cpp' and 'uiDialogVariations.cpp'.
Are you still working on this patch?

gp...@googlecode.com

unread,
Mar 2, 2012, 5:33:31 PM3/2/12
to gp...@googlegroups.com

Comment #14 on issue 68 by 00a...@gmail.com: Respect color naming

Yes, I am. I'd used macros to reduce code duplication, but it was looking a
bit ugly so I didn't want to commit it. Latest patch attached FYI; I'll
rework this to use the class instead.

Attachments:
tcnaming-rev7.diff 24.8 KB

gp...@googlecode.com

unread,
Mar 9, 2012, 1:46:44 AM3/9/12
to gp...@googlegroups.com

Comment #15 on issue 68 by 00a...@gmail.com: Respect color naming

Just dropping a note here that things are pretty busy in RL for me right
now; it could be a couple of weeks before I have time to finish this off.

gp...@googlecode.com

unread,
Jul 6, 2012, 3:13:00 AM7/6/12
to gp...@googlegroups.com

Comment #16 on issue 68 by 00a...@gmail.com: Respect color naming
I see that in the mean-time, you have almost fixed this bug -- only
uiDialogMix remains. I'll have a patch for that done shortly (within the
week).

gp...@googlecode.com

unread,
Jul 10, 2012, 1:50:07 AM7/10/12
to gp...@googlegroups.com

Comment #17 on issue 68 by thez...@gmail.com: Respect color naming
Actually there is at least two more places where this is not yet fixed:
uiDialogGenerate and PaletteFromImage. I also think that this option should
affect the names of picked colors.

gp...@googlecode.com

unread,
Aug 17, 2012, 11:28:42 PM8/17/12
to gp...@googlegroups.com

Comment #18 on issue 68 by 00a...@gmail.com: Respect color naming
I've implemented this for 'blend' and 'mix'.
'color scheme window' and 'generate scheme' are partly done.

I'm looking for ideas on what the tool specific naming for 'Palette From
Image' should be.
Currently I've got just an index into the list of generated colors. Other
ideas I have include "index %d, from %d colors".

gp...@googlecode.com

unread,
Aug 19, 2012, 5:09:45 PM8/19/12
to gp...@googlegroups.com

Comment #19 on issue 68 by thez...@gmail.com: Respect color naming
I think that a short file name and index would be nice, e.g. "image.png
color 15".

gp...@googlecode.com

unread,
Aug 21, 2012, 9:45:20 PM8/21/12
to gp...@googlegroups.com

Comment #20 on issue 68 by 00a...@gmail.com: Respect color naming
Here's a patch which implements the remaining things that were missing.

* improve 'blend colors' naming
* Color picking respects preference
* Mix dialog respects preference
* Both 'Generate scheme' method (the tab/window, and the dialog) respect
preference
* Palette From Image respects preference.

It still needs a bit of cleanup (in particular, I haven't figured out, for
Blend and Mix, whether m_is_color_item needs to be there, or I can remove
it.)


Attachments:
tool_color_naming_v9.diff 16.8 KB

gp...@googlecode.com

unread,
Aug 22, 2012, 5:18:23 PM8/22/12
to gp...@googlegroups.com

Comment #21 on issue 68 by thez...@gmail.com: Respect color naming
m_is_color_item is needed for Blend tool, because it is used to assign
different names to the colors in the endpoint widgets (start, middle, end).
Mix does not have any additional color widgets, so there is no need for
m_is_color_item in there.

name_a/name_b do not need to be copied, because their values are used
immediately in the assign() method, as ToolColorNameAssigner::assign()
calls getToolSpecificName() if tool specific names are enabled.

I think that the Blend tool has color mixing percentages mixed up.

gp...@googlecode.com

unread,
Aug 22, 2012, 9:56:07 PM8/22/12
to gp...@googlegroups.com

Comment #22 on issue 68 by 00a...@gmail.com: Respect color naming
Thanks, especially for spotting that Blend tool mixup.

I've changed the naming for Blend and Mix slightly, so that nodes are
clearly marked
(for nodes, rather than eg. 'Firebrick 0 blend 100 Golden tainoi', you
get 'Firebrick blend node'). I find this particularly helpful for Mix, when
the number of input colors exceed 2, it's quite necessary to be able to
spot the endpoints easily.

In the case of the color_items in Blend tool, they end up as 'start blend
node','middle blend node', and 'end blend node'. I hope that's ok.

I'm pretty happy with this patch now, so if everything looks okay to you, I
intend to commit it ASAP and close this issue.


Attachments:
tool_color_naming_v10.diff 17.2 KB

gp...@googlecode.com

unread,
Aug 23, 2012, 12:37:46 PM8/23/12
to gp...@googlegroups.com

Comment #23 on issue 68 by thez...@gmail.com: Respect color naming
Everything looks good.

gp...@googlecode.com

unread,
Aug 23, 2012, 9:23:22 PM8/23/12
to gp...@googlegroups.com
Updates:
Status: Fixed

Comment #24 on issue 68 by 00a...@gmail.com: Respect color naming
"'Firebrick 0 blend 100 Golden tainoi', you get 'Firebrick blend node'"

whoops, I meant

"'Firebrick 100 blend 0 Golden tainoi', you get 'Firebrick blend node'"

(marked as fixed.)

Reply all
Reply to author
Forward
0 new messages