Recent

Author Topic: Synedit: Too late to remove hardcoded colors and use system colors instead?  (Read 4455 times)

prof7bit

  • Full Member
  • ***
  • Posts: 161
I keep noticing instances all the time where people have left the synedit colors on their default hardcoded values (which were clearly choosen to match the default light windows colors), this has happened even in the Lazarus IDE itself (I am submitting  pull requests for every instance of this I am becoming aware of).

If I were in the position to be authorized for invasive decisions I would probably by now have changed the default hardcoded colors in TSynEdit and its surrounding components to consequently use system colors, I would do it in such a way that the 90% who use it on windows (where there exist only light color themes) would not see any difference and on dark themes it would then automagically[TM] look just as nice and natural as it does on light themes.

The question now is this: Changing such an old default value is an invasive change. Although most people would not notice anything if it is done carefully some might be forced to adjust their colors.

Contra:
  * might impact a certain fraction of existing applications

Pro:
  * Many still unfixed "Dark Theme Problems" will automatically fix themselves
  * the overwhelming majority of users would not notice any difference because
    - they are using a light theme and are not aware of it at all
    - they are already using their own colors anyways
  * people who will have to adapt to this change because they intentionally want a white synedit everywhere can do so without forcing them to rewrite any of their code, the needed changes are very limited and uncomplicated, just a few clicks in the object inspector.
  * Almost all other controls are already defaulting to system colors, this would remove an unexplained inconsistency.

So the question is: Do we dare to make this change once and for all? My answer would be a resounding yes!

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11382
  • FPC developer.
Re: Synedit: Too late to remove hardcoded colors and use system colors instead?
« Reply #1 on: September 27, 2021, 12:14:22 pm »
The problem is that system colors don't configure the highlighter.

For contrast, if you change e.g. the background colour to the theme you must also select other colors to match.

prof7bit

  • Full Member
  • ***
  • Posts: 161
Re: Synedit: Too late to remove hardcoded colors and use system colors instead?
« Reply #2 on: September 27, 2021, 12:24:34 pm »
I would also propose something I have been doing for quite a while in my own applications:

* a few more colors that span the range between clWindow and clWindowText in steps of 25%, 50%, 75% (or maybe even finer), this can be used for gutter, gutter separator, fold lines, etc., I also use it for thin grid lines, etc.

* two sets of "colorful" colors (used for highlighing or plotting curves), one of them adapted to bright themes, the other adapted to dark themes. On start of the application I compare the luminance of clWindow and clWindowText and if text is brighter than background I am on a dark theme and use the second set.


prof7bit

  • Full Member
  • ***
  • Posts: 161
Re: Synedit: Too late to remove hardcoded colors and use system colors instead?
« Reply #3 on: September 27, 2021, 02:30:53 pm »
See screenshot.

Probably too radical to add to these clAutoXxx colors to the Graphics unit, but the concept itself has proven to be very useful.

Code: Pascal  [Select][+][-]
  1. unit AutoColors;
  2.  
  3. {$mode ObjFPC}{$H+}
  4.  
  5. interface
  6.  
  7. uses
  8.   Graphics;
  9.  
  10. const
  11.   clWindowText20: TColor = clNone;
  12.   clWindowText40: TColor = clNone;
  13.   clWindowText60: TColor = clNone;
  14.   clWindowText80: TColor = clNone;
  15.  
  16.   // default color names from the VGA standard,
  17.   // will stay unmodified on light themes
  18.   clAutoAqua: TColor = clAqua;
  19.   clAutoBlack: TColor = clBlack;
  20.   clAutoBlue: TColor = clBlue;
  21.   clAutoFuchsia: TColor = clFuchsia;
  22.   clAutoGray: TColor = clGray;
  23.   clAutoGreen: TColor = clGreen;
  24.   clAutoLime: TColor = clLime;
  25.   clAutoMaroon: TColor = clMaroon;
  26.   clAutoNavy: TColor = clNavy;
  27.   clAutoOlive: TColor = clOlive;
  28.   clAutoPurple: TColor = clPurple;
  29.   clAutoRed: TColor = clRed;
  30.   clAutoSilver: TColor = clSilver;
  31.   clAutoTeal: TColor = clTeal;
  32.   clAutoWhite: TColor = clWhite;
  33.   clAutoYellow: TColor = clYellow;
  34.  
  35. type
  36.   TThemeForce = (
  37.     forceDark,
  38.     forceLight,
  39.     forceAuto
  40.   );
  41.  
  42. function Mix(A, B: TColor; Ratio: Integer): TColor;
  43. function IsDarkTheme: Boolean;
  44. procedure UpdateThemeColors(Force: TThemeForce=forceAuto);
  45.  
  46. implementation
  47.  
  48. function Mix(A, B: TColor; Ratio: Integer): TColor;
  49. var
  50.   RgbA, RgbB: LongInt;
  51.   Compl: Integer;
  52. begin
  53.   RgbA := ColorToRGB(A);
  54.   RgbB := ColorToRGB(B);
  55.   Compl := 10 - Ratio;
  56.   Result := RGBToColor(
  57.     (Red(RgbA)   * Compl + Red(RgbB)   * Ratio) div 10,
  58.     (Green(RgbA) * Compl + Green(RgbB) * Ratio) div 10,
  59.     (Blue(RgbA)  * Compl + Blue(RgbB)  * Ratio) div 10
  60.   );
  61. end;
  62.  
  63. function IsDarkTheme: Boolean;
  64. var
  65.   ColW, ColT: LongInt;
  66. begin
  67.   ColW := ColorToRGB(clWindow);
  68.   ColT := ColorToRGB(clWindowText);
  69.   Result := Red(ColW) + Green(ColW) + Blue(ColW) < Red(ColT) + Green(ColT) + Blue(ColT);
  70. end;
  71.  
  72. procedure UpdateThemeColors(Force: TThemeForce);
  73. var
  74.   Dark: Boolean;
  75. begin
  76.   case Force of
  77.     forceDark: Dark := True;
  78.     forceLight: Dark := False;
  79.     forceAuto: Dark := IsDarkTheme;
  80.   end;
  81.  
  82.   clWindowText20 := Mix(clWindow, clWindowText, 2);
  83.   clWindowText40 := Mix(clWindow, clWindowText, 4);
  84.   clWindowText60 := Mix(clWindow, clWindowText, 6);
  85.   clWindowText80 := Mix(clWindow, clWindowText, 8);
  86.  
  87.   if Dark then begin // tweaked for dark theme
  88.     clAutoBlack := clWhite;
  89.     clAutoWhite := clBlack;
  90.     clAutoAqua := Mix(clAqua, clBlack, 5);
  91.     clAutoLime := Mix(clGreen, clYellow, 4);
  92.     clAutoSilver := Mix(clSilver, clBlack, 5);
  93.     clAutoYellow := Mix(clYellow, clRed, 5);
  94.  
  95.     clAutoBlue := Mix(clBlue, clWhite, 6);
  96.     clAutoNavy := Mix(clBlue, clWhite, 4);
  97.     clAutoOlive := Mix(clOlive, clWhite, 2);
  98.     clAutoGreen := Mix(clGreen, clWhite, 2);
  99.     clAutoMaroon := Mix(Mix(clMaroon, clRed, 2), clWhite, 3);
  100.     clAutoPurple := Mix(clPurple, clWhite, 5);
  101.     clAutoTeal := Mix(clTeal, clWhite, 2);
  102.   end
  103.   else begin // defaults for light theme
  104.     clAutoBlack := clBlack;
  105.     clAutoWhite := clWhite;
  106.     clAutoAqua := clAqua;
  107.     clAutoLime := clLime;
  108.     clAutoSilver := clSilver;
  109.     clAutoYellow := clYellow;
  110.  
  111.     clAutoBlue := clBlue;
  112.     clAutoNavy := clNavy;
  113.     clAutoOlive := clOlive;
  114.     clAutoGreen := clGreen;
  115.     clAutoMaroon := clMaroon;
  116.     clAutoPurple := clPurple;
  117.     clAutoTeal := clTeal;
  118.   end;
  119. end;
  120.  
  121. initialization
  122.   UpdateThemeColors;
  123. end.
  124.  
« Last Edit: September 27, 2021, 02:44:46 pm by prof7bit »

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Synedit: Too late to remove hardcoded colors and use system colors instead?
« Reply #4 on: September 27, 2021, 03:17:09 pm »
Pro:
  * Many still unfixed "Dark Theme Problems" will automatically fix themselves
This is the sort of unfounded claim that goes nowhere.
I mean, I could advertise any change I want with "Fixes tons of other problems". If I do not name them, and do not show how it fixes them, then where is the value in the statement.

Quote
So the question is: Do we dare to make this change once and for all? My answer would be a resounding yes!
While my gut feeling is that my answer wont be that resounding, at this time my answer does not exist.

There is not even an example. If you are that sure of it, then surely you have that config already on your system?
Where is:
- the image how it looks on diff themes (you could use the sample text from the IDE options)
   the "auto colors" picture is not SynEdit
- the new code in TSynEdit, and (some) highlighters
   OR BETTER: an exported IDE config, to show how it looks? (What better use for the export button)



Also what is the exact proposal?

First of all you said:  "changed the default hardcoded colors in TSynEdit".
That is change what happens when I put a TSynEdit or a TSynPasSyn (highlighter) on a TForm in my own project.

But you also wrote: "this has happened even in the Lazarus IDE".
Actually, technically: No.
In the IDE the colors do not come from the defaults in the HL. Or at least the majority does not:  Comment, Number, Keyword, ... do not come from the defaults hardcoded in the SynEdit package.
The IDE has its own defaults for those.
<off topic>IIRC, in the IDE it even went the opposite way, some colors were system colors, which did not mix with hardcoded colors, so they got replaced.</off topic>

So which one do you propose to change
- SynEdit
- the IDE
- both

Because my reply would depend on that.



SynEdit:

"Add them" => Ok
"Replace the old" => No.

I could see them being added as a choice. But I would not entirely throw out the old ones.

I would be open to talk about which ones become default. Though if the default were to be changed, that poses the issue what happens to existing apps.



The IDE:

Any existing installation must not be touched, even if it uses the default colors.

That would mean, if a change was made, a new scheme would be added. This new scheme could then become default for new installations.
I.e no pre-existing user config. User config is kept by default when you uninstall. So people re-installing can keep their settings.

This is not a yes. But it is a "might be worth exploring the idea".

IMHO such a scheme could first be offered as a separate download ("userschemes").
This would allow feedback to be collected:
1) people approving of the colors, for their desktop defaults
2) people having the option to propose diff defaults (in terms of existing system colors)

It would also show, if there is any interest at all.




As a side note:
The rest of the IDE does not follow themes (yet).
So this would lead to a mismatch between IDE and editor.

And, but this is probably (almost certainly) a minority issue:
Not everyone wants the editor to follow their system defaults.
I have tons of VM for testing, many of them (Linux) come with dark defaults. Yet I want my editor to be light.


prof7bit

  • Full Member
  • ***
  • Posts: 161
Re: Synedit: Too late to remove hardcoded colors and use system colors instead?
« Reply #5 on: September 27, 2021, 04:41:22 pm »
My original proposal was not about the highlighter colors, it was about the synedit itself. For example there are two synedits placed in the makeresstrdlg, they just left the hardcoded colors in place. See my merge request https://gitlab.com/freepascal.org/lazarus/lazarus/-/merge_requests/19

Actually it is not clear why these have to be synedits in the first place, they do not use the highlighter anyways, two memos would have been enough, but I did not want to make this patch too intrusive, I just changed the two offending colors from hardcoded black and white to clWindow and clWindowText.

The IDE in general does adapt to dark themes quite nicely, most of the places where people used for example hardcoded backgrounds and system text colors have been fixed during the last years. If I still spot one I will make a merge request.

The fact that many users are using the light themes for the editor is probably because the IDE is shipping by default only with two usable themes, "default" and "Delphi", both of them are light, the other themes are two nostalgia themes, not really meant to be used, and one ugly joke of a theme called "twilight" which was probably added as some kind of alibi.

Now since the IDE has become quite usable on dark themes we could also think about complementing the two default light themes with two real(!) dark themes (real = ones that are actually designed with some dedication and are meant to be nice and usable). But this is probably a totally different discussion.

My example with the auto colors actually was just an illustration of what I meant regarding finding suitable default colors for highlighters. Unfortunately these are not constants in this implementation, so to integrate something like this properly into the graphics unit would probably mean to introduce a bunch of new system colors or something similar to it and expand the functionality of ColorToRGB() or GetSysColor(), and I doubt it would be easy to convince everybody to get this into the LCL. But it would immediately fix the hardcoded highlighter colors problem.
« Last Edit: September 27, 2021, 04:48:33 pm by prof7bit »

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Synedit: Too late to remove hardcoded colors and use system colors instead?
« Reply #6 on: September 27, 2021, 04:52:32 pm »
My original proposal was not about the highlighter colors, it was about the synedit itself. For example there are two synedits placed in the makeresstrdlg, they just left the hardcoded colors in place. See my merge request https://gitlab.com/freepascal.org/lazarus/lazarus/-/merge_requests/19
Ah, but then MarcoV is probably right.

Change those to follow default, and any app using a SynEdit + HL will have issues. Except if the App (like the IDE) sets all the colors to the app's default.

prof7bit

  • Full Member
  • ***
  • Posts: 161
Re: Synedit: Too late to remove hardcoded colors and use system colors instead?
« Reply #7 on: September 27, 2021, 05:01:18 pm »
Also what is the exact proposal?
Actually I wanted to test the waters, to see whether any change here had any chance to be accepted at all. It would be quite a bit of work to do it properly (and a lot of testing also) and it would be a waste of time if it turns out that it contradics a higher design philisophy and has no chance of being accepted anways.

Maybe first we could think about something like the auto-colors, it would not need to be all 16 vga colors, 8 colors would probably be enough, but it needs to be implemented in such a way that they are constants and can be used in initializers and selectable in the object inspector. This would need changes in the graphics unit.

Once these are in place we can safely replace the default colors in the highlighters with these new autocolors, change all synedit colors into system colors and then see how it looks on a variety of desktop themes. Would such a thing have any chances of success?

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Synedit: Too late to remove hardcoded colors and use system colors instead?
« Reply #8 on: September 27, 2021, 05:31:34 pm »
Also what is the exact proposal?
Actually I wanted to test the waters, to see whether any change here had any chance to be accepted at all. It would be quite a bit of work to do it properly (and a lot of testing also) and it would be a waste of time if it turns out that it contradics a higher design philisophy and has no chance of being accepted anways.

Maybe first we could think about something like the auto-colors, it would not need to be all 16 vga colors, 8 colors would probably be enough, but it needs to be implemented in such a way that they are constants and can be used in initializers and selectable in the object inspector. This would need changes in the graphics unit.

Once these are in place we can safely replace the default colors in the highlighters with these new autocolors, change all synedit colors into system colors and then see how it looks on a variety of desktop themes. Would such a thing have any chances of success?

Well as I have indicated, the current code isn't set in stone. So there is room.

1)
I'd like to keep the ability to set the old ones easily (could be a special property/component editor, that just sets all colors with a single click action).

2)
I'd like to be able to load the old lfm with the old default colors.
(see below for an idea)


Then the new defaults (in package SynEdit could be implemented).
Finding and agreeing on good defaults is another matter.


As for more system colors. That needs feedback from other developers too....

But, SynEdit use SysToRgb (or whatever it is called).
It could have its own version of that, and therefore have it's own wide range of "default" colors. ( above MAX_SYS_COLORS ?)

In fact that convertor could be a component. So it would be exchangeable depending on the need of an app.
Ideally colors could be converted at their origin, so they could be cached (should be easy in the "TSyn....Attribute" class, to have a cache of the translation).

Such a convertor component, could help with requirement (2) above: People with old apps, could just add (in code, global initialization section) a call to set the component, so it translates into existing colors. (That is way easier that opening potentially scores of forms, and change the settings in each lfm.)
That way we could do an incompatible change, and say to get the old behaviour, add the following line of code: SetDefaultSynColorComponent(TSynOldColorConvertor); // or similar.
(of course that needs good test, that every color is correctly kept / lots of work)



Above is just a draft. Still needs some good amount of thought.

 

TinyPortal © 2005-2018