Bug 185656

Summary: A x11/rxvt-unicode [PATCH] for the font width bug.
Product: Ports & Packages Reporter: clutton
Component: Individual Port(s)Assignee: Thierry Thomas <thierry>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff
none
unicode-example-utf8.txt none

Description clutton 2014-01-11 08:50:00 UTC 
A lot of fancy fonts like «monaco», «PragmataPro» wouldn't display correctly without this patch. And you'll see enormously large «approche».

http://i1.someimage.com/I5wLFQb.png

Fix: Patch attached with submission follows:
Comment 1 Edwin Groothuis freebsd_committer freebsd_triage 2014-01-11 08:50:08 UTC 
Responsible Changed
From-To: freebsd-ports-bugs->thierry

Over to maintainer (via the GNATS Auto Assign Tool)
Comment 2 Thierry Thomas freebsd_committer freebsd_triage 2014-01-12 13:39:07 UTC 
State Changed
From-To: open->closed


Indeed, there is a problem with some fonts. 

But, unfortunately, this kind of patch has been rejected by upstream. 

Plese see the FAQ at <http://pod.tst.eu/http://cvs.schmorp.de/rxvt-unicode/doc/rxvt.7.pod#Character_widths_are_not_correct>
Comment 3 clutton 2014-01-12 15:07:18 UTC 
Could we at least make this patch as an option? Till other workarounds
will come. Other terminal emulators work fine with those fonts...
Comment 4 Thierry Thomas freebsd_committer freebsd_triage 2014-01-12 15:49:22 UTC 
State Changed
From-To: closed->open


OK, I shall try to make it optional.
Comment 5 clutton 2014-01-12 16:02:33 UTC 
On Sun, 2014-01-12 at 15:50 +0000, thierry@FreeBSD.org wrote:
> Synopsis: A x11/rxvt-unicode [PATCH] for the font width bug.
> 
> State-Changed-From-To: closed->open
> State-Changed-By: thierry
> State-Changed-When: dim 12 jan 2014 15:49:22 UTC
> State-Changed-Why: 
> 
> OK, I shall try to make it optional.
> 
> 
> http://www.freebsd.org/cgi/query-pr.cgi?pr=185656

I can make this part.
Comment 6 Thierry Thomas freebsd_committer freebsd_triage 2014-01-12 16:26:32 UTC 
On sun 12 jan 14 at 17:02:33 +0100, clutton <clutton@zoho.com>
 wrote:

> > OK, I shall try to make it optional.
> > 
> > 
> > http://www.freebsd.org/cgi/query-pr.cgi?pr=185656
> 
> I can make this part.

I was thinking about something like this patch:

--- urxvt_font_width_bug.diff begins here ---
diff -urN x11/rxvt-unicode.orig/Makefile x11/rxvt-unicode/Makefile
--- x11/rxvt-unicode.orig/Makefile	2013-12-15 22:26:48.000000000 +0100
+++ x11/rxvt-unicode/Makefile	2014-01-12 17:20:48.000000000 +0100
@@ -30,7 +30,7 @@
 OPTIONS_DEFINE=	PERL XIM UNICODE3 ISO14755 COMBINING RXVT_SCROLLBAR	\
 		NEXT_SCROLLBAR XTERM_SCROLLBAR BACKSPACE_KEY DELETE_KEY	\
 		MOUSEWHEEL SMART_RESIZE IMLOCALE_FIX GDK_PIXBUF		\
-		NOTIFY 256_COLOR
+		NOTIFY 256_COLOR FONT_WIDTH_BUG
 
 SHEBANG_FILES=	${WRKSRC}/src/perl/*
 perl_OLD_CMD=	perl
@@ -52,6 +52,7 @@
 GDK_PIXBUF_DESC=	Use gdk-pixbuf for background images
 NOTIFY_DESC=		Freedesktop startup notification support
 256_COLOR_DESC=		Support for 256 colors
+FONT_WIDTH_BUG_DESC=	Workaround for a font width bug with some fonts
 
 OPTIONS_DEFAULT=PERL XIM UNICODE3 ISO14755 COMBINING RXVT_SCROLLBAR	\
 		NEXT_SCROLLBAR XTERM_SCROLLBAR BACKSPACE_KEY DELETE_KEY	\
@@ -192,11 +193,16 @@
 	@${ECHO_MSG}
 .endif
 
-.if ${PORT_OPTIONS:MIMLOCALE_FIX}
 post-patch:
+.if ${PORT_OPTIONS:MIMLOCALE_FIX}
 	${PATCH} ${PATCH_ARGS} < ${PATCHDIR}/extra-patch-imlocale
 	cd ${WRKSRC}/src && ./gentables
 .endif
+.if ${PORT_OPTIONS:MFONT_WIDTH_BUG}
+# Warning: this is a non-supported workaround
+# See http://pod.tst.eu/http://cvs.schmorp.de/rxvt-unicode/doc/rxvt.7.pod#Character_widths_are_not_correct
+	${PATCH} ${PATCH_ARGS} < ${PATCHDIR}/extra-patch-src_rxvtfont.C
+.endif
 
 pre-configure:
 	${RM} ${WRKSRC}/src/perl/kuake.orig
diff -urN x11/rxvt-unicode.orig/files/extra-patch-src_rxvtfont.C x11/rxvt-unicode/files/extra-patch-src_rxvtfont.C
--- x11/rxvt-unicode.orig/files/extra-patch-src_rxvtfont.C	1970-01-01 01:00:00.000000000 +0100
+++ x11/rxvt-unicode/files/extra-patch-src_rxvtfont.C	2014-01-12 11:19:11.000000000 +0100
@@ -0,0 +1,47 @@
+--- src/rxvtfont.C	2013-03-27 18:59:20.000000000 +0200
++++ /tmp/rxvtfont.C	2014-01-11 10:11:32.000000000 +0200
+@@ -1237,11 +1237,22 @@
+
+       FT_Face face = XftLockFace (f);
+
++/*
++ * use ascent, descent and height from XftFont *f instead of FT_Face face.
++ * this somehow reproduces the behaviour of the line height as seen on xterm.
++
+       ascent  = (face->size->metrics.ascender + 63) >> 6;
+       descent = (-face->size->metrics.descender + 63) >> 6;
+       height  = max (ascent + descent, (face->size->metrics.height + 63) >> 6);
+       width   = 0;
+
++ */
++
++      ascent  = f->ascent;
++      descent = f->descent;
++      height  = max (ascent + descent, f->height);
++      width   = 0;
++
+       bool scalable = face->face_flags & FT_FACE_FLAG_SCALABLE;
+
+       XftUnlockFace (f);
+@@ -1265,12 +1276,21 @@
+           XGlyphInfo g;
+           XftTextExtents16 (disp, f, &ch, 1, &g);
+
++/*
++ * bukind: don't use g.width as a width of a character!
++ * instead use g.xOff, see e.g.: http://keithp.com/~keithp/render/Xft.tutorial
++
+           g.width -= g.x;
+
+           int wcw = WCWIDTH (ch);
+           if (wcw > 0) g.width = (g.width + wcw - 1) / wcw;
+
+           if (width    < g.width       ) width    = g.width;
++ */
++          int wcw = WCWIDTH (ch);
++          if (wcw > 1) g.xOff = g.xOff / wcw;
++          if (width < g.xOff) width = g.xOff;
++
+           if (height   < g.height      ) height   = g.height;
+           if (glheight < g.height - g.y) glheight = g.height - g.y;
+         }
\ No newline at end of file
--- urxvt_font_width_bug.diff ends here ---

-- 
Th. Thomas.
Comment 7 clutton 2014-01-12 16:54:33 UTC 
On Sun, 2014-01-12 at 17:26 +0100, Thierry Thomas wrote:
> On sun 12 jan 14 at 17:02:33 +0100, clutton <clutton@zoho.com>
>  wrote:
> 
> > > OK, I shall try to make it optional.
> > > 
> > > 
> > > http://www.freebsd.org/cgi/query-pr.cgi?pr=185656
> > 
> > I can make this part.
> 
> I was thinking about something like this patch:
> 
> --- urxvt_font_width_bug.diff begins here ---
> diff -urN x11/rxvt-unicode.orig/Makefile x11/rxvt-unicode/Makefile
> --- x11/rxvt-unicode.orig/Makefile	2013-12-15 22:26:48.000000000 +0100
> +++ x11/rxvt-unicode/Makefile	2014-01-12 17:20:48.000000000 +0100
> @@ -30,7 +30,7 @@
>  OPTIONS_DEFINE=	PERL XIM UNICODE3 ISO14755 COMBINING RXVT_SCROLLBAR	\
>  		NEXT_SCROLLBAR XTERM_SCROLLBAR BACKSPACE_KEY DELETE_KEY	\
>  		MOUSEWHEEL SMART_RESIZE IMLOCALE_FIX GDK_PIXBUF		\
> -		NOTIFY 256_COLOR
> +		NOTIFY 256_COLOR FONT_WIDTH_BUG
>  
>  SHEBANG_FILES=	${WRKSRC}/src/perl/*
>  perl_OLD_CMD=	perl
> @@ -52,6 +52,7 @@
>  GDK_PIXBUF_DESC=	Use gdk-pixbuf for background images
>  NOTIFY_DESC=		Freedesktop startup notification support
>  256_COLOR_DESC=		Support for 256 colors
> +FONT_WIDTH_BUG_DESC=	Workaround for a font width bug with some fonts
>  
>  OPTIONS_DEFAULT=PERL XIM UNICODE3 ISO14755 COMBINING RXVT_SCROLLBAR	\
>  		NEXT_SCROLLBAR XTERM_SCROLLBAR BACKSPACE_KEY DELETE_KEY	\
> @@ -192,11 +193,16 @@
>  	@${ECHO_MSG}
>  .endif
>  
> -.if ${PORT_OPTIONS:MIMLOCALE_FIX}
>  post-patch:
> +.if ${PORT_OPTIONS:MIMLOCALE_FIX}
>  	${PATCH} ${PATCH_ARGS} < ${PATCHDIR}/extra-patch-imlocale
>  	cd ${WRKSRC}/src && ./gentables
>  .endif
> +.if ${PORT_OPTIONS:MFONT_WIDTH_BUG}
> +# Warning: this is a non-supported workaround
> +# See http://pod.tst.eu/http://cvs.schmorp.de/rxvt-unicode/doc/rxvt.7.pod#Character_widths_are_not_correct
> +	${PATCH} ${PATCH_ARGS} < ${PATCHDIR}/extra-patch-src_rxvtfont.C
> +.endif
>  
>  pre-configure:
>  	${RM} ${WRKSRC}/src/perl/kuake.orig
> diff -urN x11/rxvt-unicode.orig/files/extra-patch-src_rxvtfont.C x11/rxvt-unicode/files/extra-patch-src_rxvtfont.C
> --- x11/rxvt-unicode.orig/files/extra-patch-src_rxvtfont.C	1970-01-01 01:00:00.000000000 +0100
> +++ x11/rxvt-unicode/files/extra-patch-src_rxvtfont.C	2014-01-12 11:19:11.000000000 +0100
> @@ -0,0 +1,47 @@
> +--- src/rxvtfont.C	2013-03-27 18:59:20.000000000 +0200
> ++++ /tmp/rxvtfont.C	2014-01-11 10:11:32.000000000 +0200
> +@@ -1237,11 +1237,22 @@
> +
> +       FT_Face face = XftLockFace (f);
> +
> ++/*
> ++ * use ascent, descent and height from XftFont *f instead of FT_Face face.
> ++ * this somehow reproduces the behaviour of the line height as seen on xterm.
> ++
> +       ascent  = (face->size->metrics.ascender + 63) >> 6;
> +       descent = (-face->size->metrics.descender + 63) >> 6;
> +       height  = max (ascent + descent, (face->size->metrics.height + 63) >> 6);
> +       width   = 0;
> +
> ++ */
> ++
> ++      ascent  = f->ascent;
> ++      descent = f->descent;
> ++      height  = max (ascent + descent, f->height);
> ++      width   = 0;
> ++
> +       bool scalable = face->face_flags & FT_FACE_FLAG_SCALABLE;
> +
> +       XftUnlockFace (f);
> +@@ -1265,12 +1276,21 @@
> +           XGlyphInfo g;
> +           XftTextExtents16 (disp, f, &ch, 1, &g);
> +
> ++/*
> ++ * bukind: don't use g.width as a width of a character!
> ++ * instead use g.xOff, see e.g.: http://keithp.com/~keithp/render/Xft.tutorial
> ++
> +           g.width -= g.x;
> +
> +           int wcw = WCWIDTH (ch);
> +           if (wcw > 0) g.width = (g.width + wcw - 1) / wcw;
> +
> +           if (width    < g.width       ) width    = g.width;
> ++ */
> ++          int wcw = WCWIDTH (ch);
> ++          if (wcw > 1) g.xOff = g.xOff / wcw;
> ++          if (width < g.xOff) width = g.xOff;
> ++
> +           if (height   < g.height      ) height   = g.height;
> +           if (glheight < g.height - g.y) glheight = g.height - g.y;
> +         }
> \ No newline at end of file
> --- urxvt_font_width_bug.diff ends here ---

Those patches are self explaining. They change the urxvt font rendering
behaviour to xterm like. But ascent, descent, xterm like words will not
help to understand this.

I'm saying that your FONT_WIDTH_BUG_DESC is good. I can't see how else
it can be explained better.
Comment 8 clutton 2014-01-13 08:49:05 UTC 
Hm. I just tried using work/rxvt-unicode-9.19/doc/wcwidth.patch
It didn't help.
Comment 9 Thierry Thomas freebsd_committer freebsd_triage 2014-01-13 21:04:57 UTC 
State Changed
From-To: open->feedback


I'm sorry, but with your patch enabled, I noticed some errors during my 
tests with some fonts: 

urxvt: unable to calculate font width for 'Bitstream Vera Sans Mono:slant=0:weight=100:pixelsize=16:antialias=False:autohint=True:minspace=True', ignoring. 
urxvt: unable to calculate font width for 'Courier New:slant=0:weight=100:pixelsize=16:antialias=False:autohint=True:minspace=True', ignoring. 
urxvt: unable to calculate font width for 'Andale Mono:slant=0:weight=100:pixelsize=16:antialias=False:autohint=False:minspace=True', ignoring. 
urxvt: unable to calculate font width for 'FreeMono:slant=0:weight=100:pixelsize=16:autohint=True:minspace=True', ignoring. 

No problem without the patch.
Comment 10 clutton 2014-01-14 16:16:56 UTC 
Could you please provide your Xresources and how did you run urxvt.

I've just tried a lot of fonts with this command:
urxvt -fn "xft:Bitstream Vera Sans Mono"
and everything is ok.

Are you sure that the whole patch was applied?

=E2=86=911 rxvt-unicode-9.19/src =E2=86=92 diff -u rxvtfont.C.orig rxvtfont=
.C=20
--- rxvtfont.C.orig	2013-03-27 18:59:20.000000000 +0200
+++ rxvtfont.C	2014-01-14 18:08:20.000000000 +0200
@@ -1237,11 +1237,22 @@
=20
       FT_Face face =3D XftLockFace (f);
=20
+/*
+ * use ascent, descent and height from XftFont *f instead of FT_Face
face.
+ * this somehow reproduces the behaviour of the line height as seen on
xterm.
+
       ascent  =3D (face->size->metrics.ascender + 63) >> 6;
       descent =3D (-face->size->metrics.descender + 63) >> 6;
       height  =3D max (ascent + descent, (face->size->metrics.height +
63) >> 6);
       width   =3D 0;
=20
+ */
+
+      ascent  =3D f->ascent;
+      descent =3D f->descent;
+      height  =3D max (ascent + descent, f->height);
+      width   =3D 0;
+
       bool scalable =3D face->face_flags & FT_FACE_FLAG_SCALABLE;
=20
       XftUnlockFace (f);
@@ -1265,12 +1276,21 @@
           XGlyphInfo g;
           XftTextExtents16 (disp, f, &ch, 1, &g);
=20
+/*
+ * bukind: don't use g.width as a width of a character!
+ * instead use g.xOff, see e.g.:
http://keithp.com/~keithp/render/Xft.tutorial
+
           g.width -=3D g.x;
=20
           int wcw =3D WCWIDTH (ch);
           if (wcw > 0) g.width =3D (g.width + wcw - 1) / wcw;
=20
           if (width    < g.width       ) width    =3D g.width;
+ */
+          int wcw =3D WCWIDTH (ch);
+          if (wcw > 1) g.xOff =3D g.xOff / wcw;
+          if (width < g.xOff) width =3D g.xOff;
+
           if (height   < g.height      ) height   =3D g.height;
           if (glheight < g.height - g.y) glheight =3D g.height - g.y;
         }
zsh: exit 1     diff -u rxvtfont.C.orig rxvtfont.C
Comment 11 Thierry Thomas freebsd_committer freebsd_triage 2014-01-16 22:38:54 UTC 
On tue 14 jan 14 at 17:16:56 +0100, clutton <clutton@zoho.com>
 wrote:

> Could you please provide your Xresources and how did you run urxvt.
> 
> I've just tried a lot of fonts with this command:
> urxvt -fn "xft:Bitstream Vera Sans Mono"
> and everything is ok.

I have found how to reproduce the problem.

E.g. run urxvt with this command:

urxvt -letsp -1 -fn "xft:Liberation Mono:pixelsize=14" &

You can type many things in this new term, and everything seems OK,
untill you have to display some character which is not part of your
font.

For example, you can try to `cat unicode-example-utf8.txt' (I'm
attaching this file in case you have not).

Then, if I understand well, urxvt tries to find the missing chars among
other available fonts, and with your patched version, it outputs these
errors:

urxvt: unable to calculate font width for 'Courier New:slant=0:weight=100:pixelsize=16:antialias=False:autohint=True:minspace=True', ignoring.
urxvt: unable to calculate font width for 'Andale Mono:slant=0:weight=100:pixelsize=16:antialias=False:autohint=False:minspace=True', ignoring.
urxvt: unable to calculate font width for 'FreeMono:slant=0:weight=100:pixelsize=16:autohint=True:minspace=True', ignoring.

Of course, messages may change, according to the available fonts.

> Are you sure that the whole patch was applied?

Yes, it was.

Well, this is not a serious problem, and since this option is not
selected by default, I could commit your patch as-is anyway. Just wanted
to know if you had a idea about that.

Regards,
-- 
Th. Thomas.
Comment 12 Thierry Thomas freebsd_committer freebsd_triage 2014-01-20 21:12:33 UTC 
State Changed
From-To: feedback->suspended


Feedback received. Still thinking about the best way to fix the reported 
problem.
Comment 13 Thierry Thomas freebsd_committer freebsd_triage 2014-01-25 08:48:01 UTC 
State Changed
From-To: suspended->closed


Won't fix. 

According to the author (Marc Lehmann): 

"The patch is incorrect. rxvt-unicode requires a charcell font 
(basically a font suitable for a use in a grid such as used in 
terminals). Basically, your fonts have very wide glyphs, and urxvt has 
to accomodate them."