google-code-export / gpick

Automatically exported from code.google.com/p/gpick
2 stars 0 forks source link

Respect color naming preference where applicable #68

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
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.

Original issue reported on code.google.com by 00a...@gmail.com on 18 Feb 2012 at 1:06

Attachments:

GoogleCodeExporter commented 9 years ago
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?

Original comment by thezbyg on 20 Feb 2012 at 6:00

GoogleCodeExporter commented 9 years ago
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.

Original comment by 00a...@gmail.com on 21 Feb 2012 at 12:37

GoogleCodeExporter commented 9 years ago
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.

Original comment by 00a...@gmail.com on 21 Feb 2012 at 3:34

GoogleCodeExporter commented 9 years ago
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. 

Original comment by 00a...@gmail.com on 21 Feb 2012 at 4:43

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by 00a...@gmail.com on 21 Feb 2012 at 8:24

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by thezbyg on 21 Feb 2012 at 8:03

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by thezbyg on 21 Feb 2012 at 8:09

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by 00a...@gmail.com on 21 Feb 2012 at 10:15

GoogleCodeExporter commented 9 years ago
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.

Original comment by 00a...@gmail.com on 23 Feb 2012 at 4:43

GoogleCodeExporter commented 9 years ago
'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'.

Original comment by thezbyg on 23 Feb 2012 at 7:08

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by 00a...@gmail.com on 26 Feb 2012 at 11:59

Attachments:

GoogleCodeExporter commented 9 years ago
* and refactor to be much less copy-paste-ish

Original comment by 00a...@gmail.com on 26 Feb 2012 at 12:13

GoogleCodeExporter commented 9 years ago
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?

Original comment by thezbyg on 2 Mar 2012 at 8:33

GoogleCodeExporter commented 9 years ago
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.

Original comment by 00a...@gmail.com on 2 Mar 2012 at 10:32

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by 00a...@gmail.com on 9 Mar 2012 at 6:46

GoogleCodeExporter commented 9 years ago
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).

Original comment by 00a...@gmail.com on 6 Jul 2012 at 7:12

GoogleCodeExporter commented 9 years ago
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.

Original comment by thezbyg on 10 Jul 2012 at 5:49

GoogleCodeExporter commented 9 years ago
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". 

Original comment by 00a...@gmail.com on 18 Aug 2012 at 3:28

GoogleCodeExporter commented 9 years ago
I think that a short file name and index would be nice, e.g. "image.png color 
15".

Original comment by thezbyg on 19 Aug 2012 at 9:09

GoogleCodeExporter commented 9 years ago
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.)

Original comment by 00a...@gmail.com on 22 Aug 2012 at 1:45

Attachments:

GoogleCodeExporter commented 9 years ago
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.

Original comment by thezbyg on 22 Aug 2012 at 9:18

GoogleCodeExporter commented 9 years ago
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.

Original comment by 00a...@gmail.com on 23 Aug 2012 at 1:56

Attachments:

GoogleCodeExporter commented 9 years ago
Everything looks good.

Original comment by thezbyg on 23 Aug 2012 at 4:37

GoogleCodeExporter commented 9 years ago
"'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.)

Original comment by 00a...@gmail.com on 24 Aug 2012 at 1:23