Open roel-v opened 1 year ago
Hi Roel,
I agree with your motivation there - it's not easy to see which tab is selected. I guess the original default win32 styling theme was designed to work well on the old CRT type of displays. But, like my laptop, with a recent type of LCD display, has a much higher dpi and much more detailed color pallette - than the old CRTs could ever hope to provide. Also, I like to use a dark theme these days which makes the default win32 apps look a bit odd. I think in this environment, the old default win32 themes don't really achieve what they were originally designed for.
So, yeah, lots of potential for improving Vim here :)
I'd like to see how to change the color of the text as well as the background.
The idea of using the existing colors from TabLine and TabLineSel make sense to me. If they have been configured - then use them - otherwise, we can use sensible defaults.
Something else that I've been looking at, slightly related, is being able to darken the light background of the gui - for example, for the Windows 11 Dark application mode. Like, I noticed how Notepad++ now has a dark mode - and I quite like it.
Even the good old Notepad looks a LOT better now in dark mode on the latest version of Windows 11.
There doesn't seem to be an API to check when the user has enabled dark app mode for their desktop - but I was able to check for the existence of the flag in the users registry settings. And, I found out how to make the top title-bar dark - to fit in with the other apps.
But I'm still not sure how to paint the gui yet - it looks like you are closer to figuring that out than I am.
Oh, I should also point out some things I've learned about the code style for the Vim project - that I noticed your code would need to adopt. The curly braces should start on the next line. Also, will need follow how Vim's source code indentation works by alternately combining spaces (4 long) and tabs (8 long).
Thanks for your comments.
Vim uses InitCommonControls() to initialize the win32 controls (register window classes etc). Using InitCommonControlsEx() will make the program use comctl v6, which had/has better support for theming (at least, it gave programs a more graphically sophisticated look). In the Windows XP/Vista days, the tab control had some extra visual details that made the current tab stand out (when using comctl 6) - a yellow bar in XP, different background shading in Vista/7 (not sure about 8). Windows 10 has completely overhauled how all of this works, including deprecating a lot of the colors once supported by GetSysColor() (which is used by vim in gui_mch_get_color() by the way - I don't think this function still really does what it once did because of this, but maybe the effect is not noticeable in real world scenarios). I gave it a quick try to make vim use commonctl 6 by using InitCommonControlsEx() but that doesn't seem to do much, I think it also requires putting the right manifest into the resources etc. Many years ago I knew in much more detail than I ever wanted to know how all of this worked, but I've forgotten most of it. The reason I went with keeping InitCommonControls() and going for an owner draw tab control in this case, is that I think the owner draw route is much less invasive elsewhere in vim, and thus with fewer chances of side effects. But that aside...
Setting the text color is easy, it's a matter of calling SetTextColor() before doing the DrawTextW() (and resetting it to the old value afterwards).
I've looked into using TabLine and TabLineSel for the colors but I haven't yet wrapped my head around the whole mechanism of storing and passing on (font) attributes and gettings colors from them where needed. I think I need to use
int hlf_tps = HL_ATTR(HLF_TPS);
int hlf_tp = HL_ATTR(HLF_TP);
to get some sort of index into something on these attributes. However I'm not sure how to now go about converting this index to an RGB/COLORREF/triples of R, G, B values? I've found some places that use syn_term_attr2entry()/syn_cterm_attr2entry() to get an 'attrenty_T' which has an rgb value member, but when I try either of those, I just get NULL pointers. Is there anyone who can point me in the right direction how all this fits together and/or what I should use in this specific file (i.e., we already know here we're on win32 and that we're using the GUI, so I presume I won't need any of the terminal stuff)?
When that is sorted, there is the decision of what color to use for what. There are the existing color groups (I think that's the vim term?) TabLine, TabLineFill and TabLineSel. Upon close reading of the documentation, it seems that TabLine and TabLineSel refer to the text color and TabLineFill to the background (fill) color - although the 'preview' indicates otherwise? (see attached screenshot). Seems to me there should have been 4 colors, but there is probably a reason for why things are the way they are? Is there anyone who can either clarify or decide on what to do?
Another thing is that this seems to be getting the TabLine and TabLineSel setting of the current buffer, but I think they can be set per buffer? Would this mean that technically it could change when the user switches buffers?
Regarding dark mode, I think this is a separate issue. At first I thought that I had a 'set background=dark' line in my vimrc, but now that I looked it up, it's commented out. I don't remember why or when I did that - I must have put in the line in somewhere in the 1990's when I started using vim, but then I must have commented it out for some reason at some point. I don't know if what 'set background' does changed since then? Either way, it seems to me that Windows' dark mode could/should influence the default value of whatever it is that 'background' does, i.e. default to light or dark depending on the system setting, if the user hasn't explicitly overridden it. Or something - I don't really know enough about what the related vim settings mean to have an opinion on this. This could then in turn influence the color selection for tab backgrounds, but I don't think the tab background coloring should directly take the OS setting into account.
Sorry that these are more questions than answers - I don't have enough context of the vim codebase to really understand the impact of some choices so I'd rather get external input. Thanks.
Yeah, I found the SetTextColor() was real easy.
OK, so, comparing,
the current tabs look like this;
A minor tweak with your patch, it can look like this - (in my opinion, this would be a better default).
And, with regards to dark mode, I've got it looking like this;
So far, this is automatically switching based on the users windows desktop settings. I think that would be a good default type of behavior, and then we can allow it to be overridden by the users configuration if they want.
I'd like to try and make the grey area darker, so, the menu items, etc as well.
About the background setting;
set background = dark
This impacts the colors (and colorschemes) that are used within the text area. I dont think its used for any of the other gui components.
About GetSysColor() - I think MS are heading in a different direction away from the older theming ui stuff - the new Windows 10+ "dark mode" apps seem to pick-up the dark colors from somewhere else. They built up a lot of momentum going through XP, Vista, 7 - with their theming apis. But, now in Windows 11 the GetSysColors for the various components still return bright themed colors, even though some of the apps appear in dark mode, so there is something else going on - perhaps not fully documented yet by microsoft.
About using GetSysColor(), for example;
gui_mch_def_colors()
does work, it uses GetSysColor() to set the text area background and foreground colors. As it is now, on a default install, the text area in gVim starts up with black text on white background. This is picked up by gui_mch_def_colors(). I was able to make it reverse this by default when the desktop setting is for dark mode apps by a simple change in gui_mch_def_colors(). That worked really well. But again, this only affects the text area, it doesn't seem to impact any other gui components.
I'm thinking it would be good to have sensible defaults for the dark theme as well as for the light theme. Now, even though this is separate to using the users own configuration settings - but, I think it's a good step towards me understanding how it all works together. Also, personally, I'm not too worried about providing custom theming for it at this stage - my inclination is that if we can get some sensible defaults first I believe that would be a more useful starting point.
So, I haven't yet looked at how to get the colors from TabLine and TabLineSel. I might be mistaken, but those tabs in the gui - seem to have buffers and windows inside them - so I'm not sure if there is a level of parent buffer as well? I need to get my head around that too. I will do some investigation... :)
Yes your light and dark versions look much better than the current colors - 'just' having that would be a significant improvement. It would probably require some detection of what version of Windows is running, to leave the old behaviours on older versions of Windows? I'll leave it up to you to decide how this should all work,
Regarding the background of the toolbar - if you remove
SendMessage(s_toolbarhwnd, TB_SETSTYLE, 0,
SendMessage(s_toolbarhwnd, TB_GETSTYLE, 0, 0) & ~TBSTYLE_TRANSPARENT);
from around lines 7955 in gui_w32.c, the background color of the parent window will be shown through. Or you can use SetClassLong() with GCL_HBRBACKGROUND to set a custom brush, to make it the same as the background color for the tabs. But then you need to do something to make the icons themselves look light, e.g. replacing dark pixels with light ones on the fly... Quite a bit of work to support well.
Do you take over from here or is there anything from me that would help?
Using TabLine and TabLineSel for the GUI would be best. They are already used for the terminal and that appears to work. Currently the default only has the "bold" attribute, setting the background color should be possible. But it would need to be implemented for each type of GUI separately.
@roel-v
Do you take over from here or is there anything from me that would help?
I'm keen to do what I can on it. I think it looks like you've shared enough info for me to get something happening.
Although, I'm sure there'll be other questions that come up - so I do hope you would still be able to share your experience if I need some help at some point.
@brammool OK, thanks for confirming about TabLine and TabLineSel
I was playing around with theming the tabline in the console version;
Say for example, if yellow works for me in the console. But, I might not like how that looks in the GUI, Then, I think I would want to be able to keep yellow for the console, as well as configure something different for the GUI.
I guess the best way to do that would be to modify my colors file to have an if gui clause
if has("gui_running")
hi TabLine ...
hi TabLineFill ...
hi TabLineSel ...
hi Title ...
else
hi TabLine ...
hi TabLineFill ...
hi TabLineSel ...
hi Title ...
endif
Now, I think it was "Title" that colored the value "2" for the number of (buffers(or windows)?) in the tab. ie. In all my examples it shows "2" in the left of the first tab.
I think that might be an unneeded complication if we try to make "Title" color just a small section of the gui tab text as well?
@zewpo OK great, thank you. Do let me know if there's anything I can do that would be of assistance to you.
@brammool OK, thanks for confirming about TabLine and TabLineSel
I was playing around with theming the tabline in the console version;
Say for example, if yellow works for me in the console. But, I might not like how that looks in the GUI, Then, I think I would want to be able to keep yellow for the console, as well as configure something different for the GUI.
I guess the best way to do that would be to modify my colors file to have an if gui clause
if has("gui_running") hi TabLine ... hi TabLineFill ... hi TabLineSel ... hi Title ... else hi TabLine ... hi TabLineFill ... hi TabLineSel ... hi Title ... endif
That should not be necessary, the :highlight command has separate arguments for "cterm" (color terminal) and GUI. Unless you use 'termguicolors', which makes the GUI colors used in the terminal.
Now, I think it was "Title" that colored the value "2" for the number of (buffers(or windows)?) in the tab. ie. In all my examples it shows "2" in the left of the first tab.
I think that might be an unneeded complication if we try to make "Title" color just a small section of the gui tab text as well?
I don't recall why the Title highlight is used for the window count. Probably just seemed like a good idea when this was implemented. We could use only part of the Title highlight, e.g. the foreground color and attributes (bold, underline). If that isn't looking good enough adding a TabLineCount highlight should work.
-- If bankers can count, how come they have eight windows and only four tellers?
/// Bram Moolenaar -- @.*** -- http://www.Moolenaar.net \\ /// \\ \\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// \\ help me help AIDS victims -- http://ICCF-Holland.org ///
@roel-v
I've got a bit more time to play with this again, this is what it looks like so-far;
So, as you can see the menus can be custom drawn, but I was struggling to work out how to;
Do you have some ideas that could point me in the right direction?
My branch is here, if you want to see the code. https://github.com/zewpo/vim/tree/mswin-gvim-refresh
I got your branch but it seems something is off - when I force dark mode by setting
static int s_using_dark_theme = TRUE;
on line 340, I get the following:
As you can see, the width of all menu items is borked, and no menu coloring is done. Did you commit all required changes to the branch or do I need to change something else somewhere?
Will there be a way to set dark mode for vim only? Or will it only work when the system-wide dark mode is enabled? I see the dark mode colors are still hard coded but I presume this is while how the drawing is supposed to work is being figured out?
Re the outline - I can't test given the issue above, but what happens when you increase the rectangle of the menu area by one?
Re the background behind the tab bar - if you put
case WM_ERASEBKGND :
{
return -1;
}
in the tab wndproc, say around line 8574, the background won't be painted in the default colors, so I guess it will turn into the default background color of the window. On my machine it looks like what you'd want it to be:
But here be dragons. I'm quite unsure what is going to happen on other Windows versions, and it would not have worked on Windows XP where that area had some sort of gradient. Although maybe in Vim there aren't any situations where anything of a background would be visible in any circumstances, so then it wouldn't matter if the tab blends in with its surroundings. Overall trying to customize this background is fraught with pitfalls, as I'm sure you've found with googling this issue, with people having to resort to WM_PRINTCLIENT hacks and so forth. To make this work robustly, I would use a complete replacement for the win32 tab control, but that's quite an undertaking.
Re the left and right justify, what you mean is having the menu label on the left and the shortcut on the right, is that correct? As far as I know, the only way to do this is by doing it in two drawing calls. So first draw the label, then work out the width of the rendered glyphs for the shortcut, subtract from right of menu, draw shortcut text from there. But here too because the width of the whole menus is wonky on my machine, I can't really test or see what it's supposed to look like.
Thanks for getting back to me so quickly. That's interesting how it looks for you - yeah, its very possible I have missed something checked in - so I will double-check in detail soon . For now, the first thing that comes to mind, is that it might be a windows version thing, which will need to be sorted out too, in due course. I'm on Windows 11 22H2, [10.0.22621.1343]. What version are you on?
Yes same-ish:
Edition Windows 11 Pro Version 22H2 Installed on 01/10/2022 OS build 22621.819 Experience Windows Feature Experience Pack 1000.22636.1000.0
On Tue, Feb 28, 2023 at 2:51 PM Christopher Plewright < @.***> wrote:
Thanks for getting back to me so quickly. That's interesting how it looks for you - yeah, its very possible I have missed something checked in - so I will double-check in detail soon . For now, the first thing that comes to mind, is that it might be a windows version thing, which will need to be sorted out too, in due course. I'm on Windows 11 22H2, [10.0.22621.1343]. What version are you on?
— Reply to this email directly, view it on GitHub https://github.com/vim/vim/issues/11899#issuecomment-1448213786, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEKZ53BB7PDENLVLU3XYNTWZX7FFANCNFSM6AAAAAAUIWDZ3M . You are receiving this because you were mentioned.Message ID: @.***>
I've double checked, and my repo tree is in sync with the same code that I used to build gvim and generate those screenshots. So, the differences must be something else.
SDK version? My vcvars bat says this ; ** Visual Studio 2022 Developer Command Prompt v17.4.5 ... ... and some env vars like this; WindowsLibPath=C:\Program Files (x86)\Windows Kits\10\UnionMetadata\10.0.22621.0;C:\Program Files (x86)\Windows Kits\10\References\10.0.22621.0 WindowsSdkBinPath=C:\Program Files (x86)\Windows Kits\10\bin\ WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\ WindowsSDKLibVersion=10.0.22621.0\ WindowsSdkVerBinPath=C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\ WindowsSDKVersion=10.0.22621.0\
Maybe its picking up some of my color theme somewhere? I have set up my desktop with dark colors.
WRT menu size, I think the DPI scaling might be causing that difference, I can probably resolve it using the code that's already scaling other things in that C code file.
@roel-v
Sorry I didn't get a chance to respond to all the points in your comment yet. But, just letting you know I am still working through the details of your comment. Unfortunately, had to fix my laptop, the CPU wouldn't go above 30% speed until I removed the old battery - finding the root cause of that was rather tedious. Then I had some extra family come to stay, so the spare time I expected to have this week for playing with vim suddenly vanished. You know, life happens...
Anyway, I've set up a new Windows 11 VM, with default light background and no dpi scaling, and there my gvim looks just like the picture you showed me, the same borked menus; colors and dpi scaling, so I should be able to work on resolving that now that I can see it happening.
wrt your questions about default colours. My thought was to just use some sane default colors based on the users windows desktop settings - similar to how the gvim gtk3 version picks up the users desktop dark theme by default. Then, there is also vim's guioption "d" - also used by the gtk3 version - which maybe overrides the default somewhere - I want to explore how that works when I get a chance. So generally, I'd like to try and keep it on par with gtk3 version for the moment, at least where that is feasible.
wrt your question about windows versions. I am expecting to need to make it check windows versions in all the places it might try to do something that wont work for the user. That's certainly on my todo radar. I expect that's relatively easy, so it was something I planned to start doing after I get the difficult parts working on windows 11 to start with. I like to be confident that I can actually do the difficult parts first - then fill in the rest when I'm confident the idea is even feasible.
Also, thanks for this;
case WM_ERASEBKGND :
{
return -1;
}
That worked to get the tab bar background as I imagined it should look.
Onward...
I don't recall why the Title highlight is used for the window count. Probably just seemed like a good idea when this was implemented. We could use only part of the Title highlight, e.g. the foreground color and attributes (bold, underline). If that isn't looking good enough adding a TabLineCount highlight should work.
Separate TabLineCount
highlight would be great to have. Currently we have to be very cautious in colorschemes to not mess up tabline with Title background color.
@roel-v Hi, just a quick bump on this to say I think the menus are scaling a lot better now, and have hard-coded the mode for dark mode, just for now, so you can also play with it in dark mode, without having to change your whole desktop settings.
It might be too much to replicate everything in dark mode, like the menus and dialogs etc. Eg, I'm not sure what happened to the hover tooltip on the tool-bar icons - but I seem to have lost that currently. And, a number of things like that.
Soon, I think I'll start breaking this up into smaller bite-sized chunks, ie. so we can at least do a PR with just the tab-bar enhancement first. Then move onto the menus and the dialogs after that.
One idea in the back of my mind is to make a new gVim for windows using the new WinUI instead of Win32. But I'm not up for that at the moment. I initially just wanted to try and get it closer to what gtk3 can do with dark mode.
@habamax
I agree TabLineCount would be good instead of Title. Although, that might mean we want to also change the console version to not use Title in the tab-count anymore - I guess that is what you are referring to as where the current risk is now. Thinking about breaking existing use of Title there though, for people who have configured Title to appear there already, so it may be a "breaking" enhancement - something to think through carefully. Not sure how widely used it is, so don't know how big an issue that might, would want to get some feedback. Me personally, I think Title is a weird highlight name to use there in the tab-number of windows (or was it buffers? I get mixed up), and so I agree it would be a good enhancement if we can do it.
Thinking about breaking existing use of Title there though, for people who have configured Title to appear there already, so it may be a "breaking" enhancement - something to think through carefully.
It could be as usual, backward compatible with :hi link TabLineCount Title
.
@habamax
it could be as usual, backward compatible with :hi link TabLineCount Title.
yes, good point, I like an easy solution. :)
Is there any chance this will get picked up?
yes it is. please creat a PR for it
I don't have code ready to go but I can make it. My proposal would be:
As to how to do this - is there an example somewhere of how I get the 'properties' of a highlight rule? If it's just the name of a function or family of functions or what header to start looking, that'd be great.
How is this still not fixed?? Windows 10 came out eight years ago!
How is this still not fixed?? Windows 10 came out eight years ago!
Well, still waiting for a proper PR hint
Sorry I dropped the ball on this. I'm unable to pick it up again any time soon.
Microsoft in its infinite wisdom has decided that on modern Windows versions (11, at least), the background colors of selected and not selected tabs (when using the standard common controls w32 tab control, which vim does) should be only a tiny bit different. This makes it quite difficult to see which tab is selected and even more difficult what changes wrt tab selection when switching between tabs with the keyboard. There is a way to change background colors for console tabs (TabLine and TabLineSel in the color scheme), but the Windows GUI version doesn't use that. It would be nice to have a way to make the current tab stand out more.
I have attached a small proof of concept patch that will highlight the current tab in (a quite unusable for now) red. Maybe a real patch would use the colors specified in TabLine and TabLineSel? Or maybe there should be a separate setting somewhere? Or maybe it should be possible to enable/disable highlighting background colors in GUI tabs all together? There are design questions to be answered before making a real patch, hence me opening this issue.
As you can see, it's a rather minor change to draw tab backgrounds in a custom color. From what I can tell, there don't seem to be many risks wrt impact on other code or other side effects, but I haven't done much testing.
So I'm requesting feedback on if and how I should proceed with implementing this.
w32_tab_background_color.patch