Skip to content
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

Selected Fl_Menu_Item's are not drawn with selection color background under Windows #969

Closed
ManoloFLTK opened this issue May 6, 2024 · 8 comments
Assignees
Labels
fixed The issue or PR was fixed.

Comments

@ManoloFLTK
Copy link
Contributor

Describe the bug
The FLTK colormap differs between the Windows platform and other platforms. One consequence is that FL_SELECTION_COLOR (or 15) is considered by function fl_contrast() to contrast with FLTK's default background color FL_GRAY (=49) under non Windows, but not under Windows. A one step further consequence is thread
"FLTK 1.4 Menu Bar Style" of fltk.general showing that selected menu items are not drawn with the same
color in Windows and other platforms with FLTK 1.4.

To Reproduce
This image shows FL_SELECTION_COLOR on 3 platforms: X11, macOS, Windows from left to right:

(Fl_Color)15

Clearly, the Windows color is less dark and less pure blue.

Expected behavior
Is there a reason for Windows to use markedly different colors in its colormap?

FLTK Version
Please complete the following information and delete non-applicable lines:

  • Version: 1.4
  • If from Git, branch: master
  • If from Git, commit: May 6, 2024
@ManoloFLTK ManoloFLTK added the Platform: Windows platform specific (Windows) label May 6, 2024
@MatthiasWM
Copy link
Contributor

Yes, it's intentional that different platforms can have different palettes. It's not necessarily intentional that fl_contrast "flips" a color scheme so badly, that the result is less pleasing/readable.

The original idea was to adjust the palettes to the color scheme of the desktop in use. The user can override this with command line options during Fl::args(args, argv) or Fl_Window::show(args, argv). The macOS version actually reads the desktop color scheme (early macOS had more than just one). You can open a macOS Fl_Window with show(args, argv) and it will be slightly brighter than with just show(). However all this was introduced before dark mode became a thing, and we assumed that the background is always relatively bright and fonts are relatively dark, so dark modes don't work at all. Also, fl_contrast() was changed (for the better).

So all in all, yes, this is intentional and well meant. It may have more of an influence on colors than was originally intended. We have plans to fix all this in 1.5 .

@ManoloFLTK
Copy link
Contributor Author

OK. I would think we should find a way to remove such a big difference in the way selected menu items are drawn on Windows (black text on white background) relatively to other platforms (white text on blue background).

@Albrecht-S
Copy link
Member

Albrecht-S commented May 6, 2024

I would think we should find a way to remove such a big difference in the way selected menu items are drawn on Windows (black text on white background) relatively to other platforms (white text on blue background).

I'm investigating this in the context of the mentioned thread in fltk.general. As a first thought: if the background color of the selected menu item is "flipped" then this is not intended. The only thing that should be changed by fl_contrast() is the foreground (text) color, depending on the selected background color. If this is not the case, i.e. if the background color of the selected menu item is changed, then I believe that this is a bug: fl_contrast() used in the wrong place or with wrong arguments. We need to check this in the menu code.

Side note: a long time ago one of my customers used a "Knoppix" Linux system with KDE desktop (IIRC). The default selection color was a really light blue. In this case fl_contrast() didn't work well and used a white foreground (text) color on the light blue selection (background) color. This was due to the simple but incorrect contrast calculation algorithm used in 1.3. The new default fl_contrast() algorithm in 1.4 is much better (objectively) but can yield different results than the one used in 1.3 (obviously, that's why it was changed). It depends largely on the background (or selection) color chosen by the system or theme whether the result is the same as in 1.3 or not.

@dannye
Copy link

dannye commented May 7, 2024

As a first thought: if the background color of the selected menu item is "flipped" then this is not intended. The only thing that should be changed by fl_contrast() is the foreground (text) color, depending on the selected background color.

I am very much in favor of this approach.

@ManoloFLTK
Copy link
Contributor Author

ManoloFLTK commented May 7, 2024

Here is the code in function Fl_Menu_Item::draw() of src/Fl_Menu.cxx that sets the text and background colors to draw menu items. This code uses fl_contrast() twice which, I believe, is ultimately too much.

 if (fl_contrast(r, color) != r) { // back compatibility boxtypes
     if (selected == 2) { // menu title
       r = color;
       b = m ? m->box() : FL_UP_BOX;
     } else {
       r = (Fl_Color)(FL_COLOR_CUBE-1); // white
       l.color = fl_contrast((Fl_Color)labelcolor_, r);
     }
   } else {
     l.color = fl_contrast((Fl_Color)labelcolor_, r);
   }

Replacing this by just
l.color = fl_contrast((Fl_Color)labelcolor_, r);
fixes the issue of Windows menu items, keeps the lighter selection hue used in Windows, and makes sure the text is drawn with enough contrast relatively to its background with a single fl_contrast() call.

Added later: it might be necessary to add some more code to draw properly menu titles.

@ManoloFLTK
Copy link
Contributor Author

@Albrecht-S I'm left with the impression you're willing to take care of this issue. If that's not correct, please, let it known here.

@dannye
Copy link

dannye commented May 7, 2024

I propose re-titling this to something like "Fl_Menu_Item selection color contrast bug" or something similar, and removing the Platform: Windows label, and perhaps rewriting the description to not really focus on the default OS colormaps. The Windows colormap is just simply one victim of the broader underlying bug.

@Albrecht-S
Copy link
Member

I'm left with the impression you're willing to take care of this issue. If that's not correct, please, let it known here.

@ManoloFLTK I won't be able to work on this for at least the next two or three days, and even longer if I do my remaining CMake and Windows keyboard stuff before that (which I intend to do).

I would also appreciate if @MatthiasWM could contribute some knowledge about the menu handling code. Maybe he knows a reason why this "color flipping" of the background/menu item has been done.

That said, I'd be grateful if someone else could take care of the code you posted above to avoid "color flipping" of selected menu items in the first place. I also wouldn't mind if this issue was renamed to a more appropriate name, as @dannye suggested above. Please feel free to assign yourself and work on a solution. TIA.

@ManoloFLTK ManoloFLTK self-assigned this May 8, 2024
@ManoloFLTK ManoloFLTK changed the title The FLTK colormap differs between the Windows platform and other platforms Selected Fl_Menu_Item's are not drawn with selection color background under Windows May 8, 2024
ManoloFLTK added a commit that referenced this issue May 8, 2024
Also, remove this text from the doc of Fl_Menu_::down_box()
    "If this is FL_NO_BOX then it acts like
    FL_THIN_UP_BOX and selection_color() acts like
    FL_WHITE, for back compatibility."
that was true only for the Windows platform and that required
selection_color to be replaced by white for menu items which is
not what FLTK 1.4 expects.

The new state of menu item drawings is as follows :
- all platforms draw menu items with the same symbolic colors
- selected items and menu titles are drawn with the selection color
as background color
- menu items are drawn by default with no box but can be given one
by Fl_Menu_::down_box(Fl_Boxtype)
- the text of selected items is drawn with the menu's color
unless fl_contrast() finds it does not make enough contrast with
the background (selection color) and substitutes it with a more
adapted color
- the Windows platform uses a visibly different hue for its default
selection color from what other platforms use
@ManoloFLTK ManoloFLTK added the fixed The issue or PR was fixed. label May 8, 2024
@Albrecht-S Albrecht-S removed the Platform: Windows platform specific (Windows) label May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed The issue or PR was fixed.
Projects
None yet
Development

No branches or pull requests

4 participants