Skip to content

CSSTUDIO-3196 Change the semantics of colors defined to be equal to other colors #3376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abrahamwolk
Copy link
Collaborator

Consider the color definitions

GREEN = 61, 216, 61
OK = GREEN

Currently, this will result in Phoebus having the following two colors defined:

GREEN = 61, 216, 61
OK = 61, 216, 61

The intent of this pull request is to change the semantics so that the color definitions instead results in only the color

GREEN = 61, 216, 61

being defined, while any reference to the color "OK" is replaced with a reference to the color "GREEN" instead. (I.e., "OK" points to the the color GREEN instead of being defined as an identical color.)

The motivation for this change is that widgets in Phoebus have hard-coded default colors, and setting them all to default values results in identical colors being defined that differ only in their name. With this change, it is possible to instead "redirect" the default colors to other colors.

…ther colors: a color defined to be equal to another color is now an alias for the other color.
@kasemir
Copy link
Collaborator

kasemir commented Apr 22, 2025

When I open the color selection dialog, do I still see "OK" and "GREEN" listed in there?

For an LED that displays a good/bad state, I want to pick "OK".
For a rectangle that represents a tank full of green slime, I want to pick "GREEN".
Right now they might give me the same color, and it's great to be able to define "OK=GREEN" so OK is indeed the exact same color as GREEN.
But "GREEN" must always remain green, while "OK" could over time be defined as bright green, dull green, dark red or gray like a fault LED that's off because we're OK and so on.

@abrahamwolk
Copy link
Collaborator Author

abrahamwolk commented Apr 23, 2025

When I open the color selection dialog, do I still see "OK" and "GREEN" listed in there?

No, and that is the purpose of the pull request: to reduce the number of duplicated colors in the list of colors.

For an LED that displays a good/bad state, I want to pick "OK". For a rectangle that represents a tank full of green slime, I want to pick "GREEN". Right now they might give me the same color, and it's great to be able to define "OK=GREEN" so OK is indeed the exact same color as GREEN. But "GREEN" must always remain green, while "OK" could over time be defined as bright green, dull green, dark red or gray like a fault LED that's off because we're OK and so on.

The intended scenario is that the default colors are defined to be "correct" according to a convention defined in a written document. Only seldomly, in rare cases, should the default colors be changed.

Suppose that one of those rare cases happens, and the user wants to select a green color. The intent of the pull request is to avoid the situation where there are several (duplicated or different) green colors.

A concrete example: Consider the LED widget. Right now, if you don't actively pick a color, the on-state has the color 'On'. When starting Phoebus, there is also the color 'OK' defined. The two colors 'On' and 'OK' may be identical or not. When choosing a green color to signal that something is "on" or "OK", it is not always intuitively clear which color to choose, since it may not be so clear which of the adjectives more accurately describes the status.

@abrahamwolk
Copy link
Collaborator Author

Of course, you are right that if you want two defined colors that are identical, then, with this change, the color would have to be defined twice using the numerical values. In the example in the initial comment, one would have to write:

GREEN = 61, 216, 61
OK = 61, 216, 61

@kasemir
Copy link
Collaborator

kasemir commented Apr 23, 2025

the purpose of the pull request: to reduce the number of duplicated colors in the list of colors

Isn't that a site-specific decision?
You create the colors.def for your site, and in there you are free to put whatever you want.
You can only have an "OK" color, or you can have "OK", "On", "Green", "Good".
Why should the implementation second-guess what you put into your site specific color definitions and simply drop some of them?!

@abrahamwolk
Copy link
Collaborator Author

Isn't that a site-specific decision?
You create the colors.def for your site, and in there you are free to put whatever you want.
You can only have an "OK" color, or you can have "OK", "On", "Green", "Good".
Why should the implementation second-guess what you put into your site specific color definitions and simply drop some of them?!

Phoebus does also create colors that are not specified in colors.def. There are at least two ways in which the code of Phoebus creates colors, one resulting in colors being added to the list of colors and one which results in colors being added to the list of colors only when they are set in colors.def:

  1. In the method defineDefaultColors() in the class NamedWidgetsColors. E.g., the color "OK" is defined here. (The constant ALARM_OK equals "OK".)
  2. Widgets define default colors. E.g., the LED widget defines its default color to be the color new NamedWidgetColor("On", 0, 255, 0). This does not automatically result in the color "On" being added to the list of colors; however, if the color "On" is re-defined in colors.def, then "On" does appear in the list of colors.

I am not sure whether the solution in the pull request is the best way to solve this. However, I think that it would be good to have a way of setting default colors for widgets without creating new colors.

@abrahamwolk
Copy link
Collaborator Author

Isn't that a site-specific decision?
You create the colors.def for your site, and in there you are free to put whatever you want.
You can only have an "OK" color, or you can have "OK", "On", "Green", "Good".

The problem is that this is not the case: the widgets (through the default colors) and alarm-functionality dictate colors that appear in the list of colors, at the latest after they have been configured in colors.def.

The intention of this pull request is to allow the names of the colors that appear in the list of colors to be site-specific.

Perhaps it would be better to introduce a second mechanism for aliasing colors?

Currently one can write, e.g, GREEN = 61, 216, 61 to define a new color, and OK = GREEN to define a new color that has the same numerical values as GREEN. We could introduce a new syntax that lets one redirect a name to a different name, e.g. (I don't know if this is the best syntax): OK = alias(GREEN).

@shroffk
Copy link
Member

shroffk commented Apr 29, 2025

@abrahamwolk
I think we can look at default colors which are defined in widgets and applications and come up with a way to ensure they are consistently visible in all color selection dialogs.

I do not like the idea of aliasing or automatically trimming the color list...I think this is a problem that is best resolved by maintaining a well defined color.def file.

I am happy to discuss a solution for the first ( since I am guilty of creating the alarm level colors which do not show up in the color selection dialogs. )

@abrahamwolk
Copy link
Collaborator Author

@shroffk I think it would be great to have a meeting to discuss this. I will send you an invitation.

@kasemir
Copy link
Collaborator

kasemir commented Apr 30, 2025

As long as I can do this...

.. with this change, the color would have to be defined twice using the numerical values..
GREEN = 61, 216, 61
OK = 61, 216, 61

.. you can do whatever you prefer regarding OK=GREEN or a new alias syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants