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

Add weight specifier for font specification for Windows. #4309

Closed
wants to merge 5 commits into from

Conversation

charlton1
Copy link

@charlton1 charlton1 commented Apr 27, 2019

get_logfont() always uses FW_NORMAL (400) as a baseline. Without the ability to
specify an explicit weight, the Windows fontmapper will often make
Light or ExtraLight variants inaccessible. Using the font chooser
does not typically have this problem because the precise font
weight is specified in the LOGFONT structure. There is currently
no way to get equivalent functionality via the guifont (gfn)
setting. This commit adds the 'W' attribute to a font specifier which
allows for the indication of an explicit font weight.

set guifont=My_Awesome_Font:h13:W275

This fix is intended to address #1723

always uses FW_NORMAL (400) as a baseline. Without the ability to
specify an explicit weight, the Windows fontmapper will often make
Light or ExtraLight variants inaccessible. Using the font chooser
does not typically have this problem because the precise font
weight is specified in the LOGFONT structure. There is currently
no way to get equivalent functionality via the guifont (gfn)
setting.
@codecov-io
Copy link

codecov-io commented Apr 27, 2019

Codecov Report

Merging #4309 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4309      +/-   ##
==========================================
+ Coverage   79.92%   79.93%   +<.01%     
==========================================
  Files         108      108              
  Lines      141702   141702              
==========================================
+ Hits       113262   113270       +8     
+ Misses      28440    28432       -8
Impacted Files Coverage Δ
src/gui.c 57.63% <0%> (-0.32%) ⬇️
src/channel.c 83.47% <0%> (-0.08%) ⬇️
src/term.c 69.1% <0%> (-0.06%) ⬇️
src/screen.c 80.62% <0%> (+0.02%) ⬆️
src/terminal.c 80.66% <0%> (+0.03%) ⬆️
src/os_unix.c 59.39% <0%> (+0.04%) ⬆️
src/getchar.c 76.45% <0%> (+0.04%) ⬆️
src/ex_cmds2.c 87.31% <0%> (+0.11%) ⬆️
src/gui_gtk_x11.c 49.04% <0%> (+0.39%) ⬆️
src/if_xcmdsrv.c 84.02% <0%> (+0.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dcaa54d...decbf49. Read the comment docs.

Copy link
Member

@k-takata k-takata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'guifont' part of runtime/doc/options.txt should be also updated.

src/os_mswin.c Outdated Show resolved Hide resolved
@brammool brammool added the needs more work used for a pull request that isn't ready to include (other than needing a test) label Apr 27, 2019
@brammool
Copy link
Contributor

brammool commented Apr 27, 2019 via email

@charlton1
Copy link
Author

charlton1 commented Apr 27, 2019 via email

documentation to inform about new weight specification and to
clarify that a bold ('b') attribute is equivalent to a 700
weight
@charlton1
Copy link
Author

charlton1 commented Apr 27, 2019 via email

@brammool
Copy link
Contributor

brammool commented Apr 27, 2019 via email

@charlton1
Copy link
Author

charlton1 commented Apr 27, 2019 via email

@k-takata
Copy link
Member

I think it's better to update also logfont2name() in gui_w32.c.
E.g.:

--- a/src/gui_w32.c
+++ b/src/gui_w32.c
@@ -3125,6 +3125,7 @@ logfont2name(LOGFONTW lf)
     char	*charset_name;
     char	*quality_name;
     char	*font_name;
+    int		points;
 
     font_name = (char *)utf16_to_enc(lf.lfFaceName, NULL);
     if (font_name == NULL)
@@ -3132,15 +3133,19 @@ logfont2name(LOGFONTW lf)
     charset_name = charset_id2name((int)lf.lfCharSet);
     quality_name = quality_id2name((int)lf.lfQuality);
 
-    res = (char *)alloc((unsigned)(strlen(font_name) + 20
+    res = (char *)alloc((unsigned)(strlen(font_name) + 30
 		    + (charset_name == NULL ? 0 : strlen(charset_name) + 2)
 		    + (quality_name == NULL ? 0 : strlen(quality_name) + 2)));
     if (res != NULL)
     {
 	p = res;
 	/* make a normal font string out of the lf thing:*/
-	sprintf((char *)p, "%s:h%d", font_name, pixels_to_points(
-			 lf.lfHeight < 0 ? -lf.lfHeight : lf.lfHeight, TRUE));
+	points = pixels_to_points(
+			 lf.lfHeight < 0 ? -lf.lfHeight : lf.lfHeight, TRUE);
+	if (lf.lfWeight == FW_NORMAL || lf.lfWeight == FW_BOLD)
+	    sprintf((char *)p, "%s:h%d", font_name, points);
+	else
+	    sprintf((char *)p, "%s:h%d:W%d", font_name, points, lf.lfWeight);
 	while (*p)
 	{
 	    if (*p == ' ')
@@ -3149,7 +3154,7 @@ logfont2name(LOGFONTW lf)
 	}
 	if (lf.lfItalic)
 	    STRCAT(p, ":i");
-	if (lf.lfWeight >= FW_BOLD)
+	if (lf.lfWeight == FW_BOLD)
 	    STRCAT(p, ":b");
 	if (lf.lfUnderline)
 	    STRCAT(p, ":u");

src/os_mswin.c Outdated Show resolved Hide resolved
src/os_mswin.c Outdated Show resolved Hide resolved
@charlton1
Copy link
Author

charlton1 commented Apr 28, 2019 via email

is included in the name. Delete extraneous whitespace. Convert
call to wcstol to use only base 10 instead of auto-detect.
@charlton1
Copy link
Author

charlton1 commented Apr 28, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more work used for a pull request that isn't ready to include (other than needing a test) platform-windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants