On Mon, 2015-06-15 at 10:25 +0200, Roberto E. Vargas Caballero wrote:
> Hi,

Well, it's been a really long time and I thought I'd finally get around
to summarizing what was agreed upon way back when.

>       I like the idea, but I think the patch needs some evolution.
> A patch of 500 lines is usually hard of reading, and in in this case
> the change is not trivial.
> 
> On Wed, Jun 10, 2015 at 11:39:25PM +0200, Joakim Sindholt wrote:
> > glyph now holds a union of two combining characters and a pointer to a zero
> > terminated array. This is purely to make the most of the space. No length is
> > kept and every character added past 2 causes a realloc.
> 
> And why 2 characters?. I think it is more logical to have only one, which is
> the most common case, isn't it?. Maybe I'm missing something.

Because of space. I would wager that most computers have 64-bit pointers
nowadays, so to optimally use the space before going full malloc, we can
have one base character plus two combining characters. However one base
plus one combining is by far the most common in western scripts.

> > There's currently a problem with rendering where Xft doesn't draw the
> > combining characters as a part of the character they combine to, which 
> > causes
> > things like Hangul Jamo to be completely unusable stacks of characters 
> > instead
> > of the proper combined form.
> 
> Uhmm, I don't understand this point, can you explain it a bit better?,
> or better, put an example.

I used 㐻o͜ò떫 as my test characters (see http://sprunge.us/BIac if it
didn't render right for you). It should be a double width Chinese
character, followed by two o's, with a double width combining horizontal
end parenthesis under them both (not just the left one as with my
patch) and a ` over the second one, followed by a combining Hangul Jamo
character (not a box with a line through it as it renders with my
patch).

Since I really don't have time to tackle this properly, I'll leave
anyone who wants to pick up the torch with the same patch updated to git
head. Maybe it could be put on the wiki or something. Sorry for wasting
everyone's time with this.

I would say the criticisms below are spot on:

> > @@ -98,6 +98,7 @@ enum glyph_attribute {
> >     ATTR_WRAP       = 1 << 8,
> >     ATTR_WIDE       = 1 << 9,
> >     ATTR_WDUMMY     = 1 << 10,
> > +   ATTR_DECOMPPTR  = 1 << 11,
> >     ATTR_BOLD_FAINT = ATTR_BOLD | ATTR_FAINT,
> >  };
> >  
> > @@ -190,6 +191,10 @@ typedef XftColor Color;
> >  
> >  typedef struct {
> >     Rune u;           /* character code */
> > +   union {
> > +           Rune c[2];
> > +           Rune *pc;
> > +   } dc;
> 
> We already have the field u, so why is not the union made with u and pc?
> 
> >     Line *alt;    /* alternate screen */
> >     bool *dirty;  /* dirtyness of lines */
> >     XftGlyphFontSpec *specbuf; /* font spec buffer used for rendering */
> > +   int specbuflen;
> 
> 
> > @@ -419,8 +426,9 @@ static void ttywrite(const char *, size_t);
> >  static void tstrsequence(uchar);
> >  
> >  static inline ushort sixd_to_16bit(int);
> > -static int xmakeglyphfontspecs(XftGlyphFontSpec *, const Glyph *, int, 
> > int, int);
> > -static void xdrawglyphfontspecs(const XftGlyphFontSpec *, Glyph, int, int, 
> > int);
> > +static int xmakeglyphfontspecs(XftGlyphFontSpec *, int, const Glyph *, 
> > int, int, int);
> > +static int xmakeglyphfontspecsintermbuf(const Glyph *, int, int, int);
> > +static void xdrawglyphfontspecs(const XftGlyphFontSpec *, Glyph, int, int, 
> > int, int);
> 
> Uhmmm, these names begin to seem like java names. We must find a better way 
> to name them,
> because with names so long the only lower case style doesn't work.
> 
> >     /* append every set & selected glyph to the selection */
> > @@ -984,6 +993,16 @@ getsel(void) {
> >                             continue;
> >  
> >                     ptr += utf8encode(gp->u, ptr);
> > +                   if(gp->mode & ATTR_DECOMPPTR) {
> > +                           pc = gp->dc.pc;
> > +                           for(pc = gp->dc.pc; *pc; pc++)
> > +                                   ptr += utf8encode(*pc, ptr);
> > +                   } else {
> > +                           if(gp->dc.c[0])
> > +                                   ptr += utf8encode(gp->dc.c[0], ptr);
> > +                           if(gp->dc.c[1])
> > +                                   ptr += utf8encode(gp->dc.c[1], ptr);
> > +                   }
> >             }
> 
> If we define the union with u and pc we can remove the else in this code, no?
> 
> > +   term.dirty[y] = 1;
> > +   if(term.line[y][x].mode & ATTR_DECOMPPTR) {
> > +           p = term.line[y][x].dc.pc;
> > +           while(*p)
> > +                   p++;
> > +           sz = (p - term.line[y][x].dc.pc) + 2;
> > +           term.line[y][x].dc.pc = xrealloc(term.line[y][x].dc.pc, sz * 
> > sizeof(Rune));
> > +           term.line[y][x].dc.pc[sz-2] = u;
> > +           term.line[y][x].dc.pc[sz-1] = 0;
> > +   } else {
> > +           if(!term.line[y][x].dc.c[0]) {
> > +                   term.line[y][x].dc.c[0] = u;
> > +           } else if(!term.line[y][x].dc.c[1]) {
> > +                   term.line[y][x].dc.c[1] = u;
> > +           } else {
> > +                   p = xmalloc(4 * sizeof(Rune));
> > +                   p[0] = term.line[y][x].dc.c[0];
> > +                   p[1] = term.line[y][x].dc.c[1];
> > +                   p[2] = u;
> > +                   p[3] = 0;
> > +                   term.line[y][x].dc.pc = p;
> > +                   term.line[y][x].mode |= ATTR_DECOMPPTR;
> > +           }
> > +   }
> > +}
> 
> The same. If the union is made with u almost all off this code disappear.
> 
> > @@ -1711,7 +1786,7 @@ tclearregion(int x1, int y1, int x2, int y2) {
> >  
> >  void
> >  tdeletechar(int n) {
> > -   int dst, src, size;
> > +   int dst, src, size, i;
> >     Glyph *line;
> >  
> >     LIMIT(n, 0, term.col - term.c.x);
> > @@ -1721,13 +1796,17 @@ tdeletechar(int n) {
> >     size = term.col - src;
> >     line = term.line[term.c.y];
> >  
> > +   for(i = dst; i < src; i++) {
> > +           if(line[i].mode & ATTR_DECOMPPTR)
> > +                   line[i].dc.pc = NULL;
> > +   }
> 
> ...
> 
> > @@ -1737,6 +1816,10 @@ tinsertblank(int n) {
> >     size = term.col - dst;
> >     line = term.line[term.c.y];
> >  
> > +   for(i = src; i < dst; i++) {
> > +           if(line[i].mode & ATTR_DECOMPPTR)
> > +                   line[i].dc.pc = NULL;
> > +   }
> >     memmove(&line[dst], &line[src], size * sizeof(Glyph));
> >     tclearregion(src, term.c.y, dst - 1, term.c.y);
> >  }
> 
> 
> Maybe we should write a blank in u here, because if we have NULL and blank
> everything becomes harder.
> 
> 
> > @@ -2774,6 +2857,19 @@ tputc(Rune u) {
> >     if(sel.ob.x != -1 && BETWEEN(term.c.y, sel.ob.y, sel.oe.y))
> >             selclear(NULL);
> >  
> > +   /* combining chars are added to the previous cell */
> > +   if(width == 0) {
> > +           if(term.c.x == 0) {
> > +                   if(term.c.y == 0)
> > +                           taddcchar(u, 0, 0);
> > +                   else
> > +                           taddcchar(u, term.col-1, term.c.y-1);
> > +           } else {
> > +                   taddcchar(u, term.c.x-1, term.c.y);
> > +           }
> > +           return;
> > +   }
> > +
> 
> You repeat this logic of term.c.x and term.c.y in several places of the code,
> maybe is better to add some function whose name is clear enough to make 
> obvious
> this logic.
> 
> > @@ -2834,12 +2936,19 @@ tresize(int col, int row) {
> >             memmove(term.alt, term.alt + i, row * sizeof(Line));
> >     }
> >     for(i += row; i < term.row; i++) {
> > +           for(j = 0; j < term.col; j++) {
> > +                   if(term.line[i][j].mode & ATTR_DECOMPPTR)
> > +                           free(term.line[i][j].dc.pc);
> > +                   if(term.alt[i][j].mode & ATTR_DECOMPPTR)
> > +                           free(term.alt[i][j].dc.pc);
> > +           }
> 
> ...
> 
> > @@ -2849,14 +2958,26 @@ tresize(int col, int row) {
> >  
> >     /* resize each row to new width, zero-pad if needed */
> >     for(i = 0; i < minrow; i++) {
> > +           for(j = col; j < term.col; j++) {
> > +                   if(term.line[i][j].mode & ATTR_DECOMPPTR)
> > +                           free(term.line[i][j].dc.pc);
> > +                   if(term.alt[i][j].mode & ATTR_DECOMPPTR)
> > +                           free(term.alt[i][j].dc.pc);
> > +           }
> 
> The same with this logic.
> 
> >  int
> > -xmakeglyphfontspecs(XftGlyphFontSpec *specs, const Glyph *glyphs, int len, 
> > int x, int y)
> > +xmakeglyphfontspecs(XftGlyphFontSpec *specs, int nspecs, const Glyph 
> > *glyphs, int len, int x, int y)
> >  {
> > -   float winx = borderpx + x * xw.cw, winy = borderpx + y * xw.ch, xp, yp;
> > +   float winx = borderpx + x * xw.cw, winy = borderpx + y * xw.ch;
> 
> I think the very long name of this function and the number of parameters are 
> very good
> indicators we are doing something wrong here, and we should do some rewriting 
> instead
> of patching the code.
> 
> > @@ -3326,11 +3464,12 @@ xmakeglyphfontspecs(XftGlyphFontSpec *specs, const 
> > Glyph *glyphs, int len, int x
> >             /* Lookup character index with default font. */
> >             glyphidx = XftCharIndex(xw.dpy, font->match, rune);
> >             if(glyphidx) {
> > -                   specs[numspecs].font = font->match;
> > -                   specs[numspecs].glyph = glyphidx;
> > -                   specs[numspecs].x = (short)xp;
> > -                   specs[numspecs].y = (short)yp;
> > -                   xp += runewidth;
> > +                   if(numspecs < nspecs) {
> > +                           specs[numspecs].font = font->match;
> > +                           specs[numspecs].glyph = glyphidx;
> > +                           specs[numspecs].x = (short)xp;
> > +                           specs[numspecs].y = (short)yp;
> > +                   }
> >                     numspecs++;
> >                     continue;
> >             }
> > @@ -3401,20 +3540,43 @@ xmakeglyphfontspecs(XftGlyphFontSpec *specs, const 
> > Glyph *glyphs, int len, int x
> >                     FcCharSetDestroy(fccharset);
> >             }
> >  
> > -           specs[numspecs].font = frc[f].font;
> > -           specs[numspecs].glyph = glyphidx;
> > -           specs[numspecs].x = (short)xp;
> > -           specs[numspecs].y = (short)(winy + frc[f].font->ascent);
> > -           xp += runewidth;
> > +           if(numspecs < nspecs) {
> > +                   specs[numspecs].font = frc[f].font;
> > +                   specs[numspecs].glyph = glyphidx;
> > +                   specs[numspecs].x = (short)xp;
> > +                   specs[numspecs].y = (short)(winy + frc[f].font->ascent);
> > +           }
> >             numspecs++;
> >     }
> >  
> >     return numspecs;
> >  }
> 
> Again, I think we could try some way to merge these two sections of code that 
> are
> identical (and they are in the same function).
> 
> 
> I would like continue the discussion of this patch, but please, if you can 
> find some
> way to split it in small patches it will make it easier my life.
> 
> Regards,
> 
> 




 



diff --git a/st.c b/st.c
index bd8b815..6b5c621 100644
--- a/st.c
+++ b/st.c
@@ -100,6 +100,7 @@ enum glyph_attribute {
 	ATTR_WRAP       = 1 << 8,
 	ATTR_WIDE       = 1 << 9,
 	ATTR_WDUMMY     = 1 << 10,
+	ATTR_DECOMPPTR  = 1 << 11,
 	ATTR_BOLD_FAINT = ATTR_BOLD | ATTR_FAINT,
 };
 
@@ -192,6 +193,10 @@ typedef XftColor Color;
 
 typedef struct {
 	Rune u;           /* character code */
+	union {
+		Rune c[2];
+		Rune *pc;
+	} dc;
 	ushort mode;      /* attribute flags */
 	uint32_t fg;      /* foreground  */
 	uint32_t bg;      /* background  */
@@ -235,6 +240,7 @@ typedef struct {
 	Line *alt;    /* alternate screen */
 	int *dirty;  /* dirtyness of lines */
 	XftGlyphFontSpec *specbuf; /* font spec buffer used for rendering */
+	int specbuflen;
 	TCursor c;    /* cursor */
 	int top;      /* top    scroll limit */
 	int bot;      /* bottom scroll limit */
@@ -402,6 +408,7 @@ static void tscrollup(int, int);
 static void tscrolldown(int, int);
 static void tsetattr(int *, int);
 static void tsetchar(Rune, Glyph *, int, int);
+static void taddcchar(Rune, int, int);
 static void tsetscroll(int, int);
 static void tswapscreen(void);
 static void tsetdirt(int, int);
@@ -422,8 +429,9 @@ static void ttywrite(const char *, size_t);
 static void tstrsequence(uchar);
 
 static inline ushort sixd_to_16bit(int);
-static int xmakeglyphfontspecs(XftGlyphFontSpec *, const Glyph *, int, int, int);
-static void xdrawglyphfontspecs(const XftGlyphFontSpec *, Glyph, int, int, int);
+static int xmakeglyphfontspecs(XftGlyphFontSpec *, int, const Glyph *, int, int, int);
+static int xmakeglyphfontspecsintermbuf(const Glyph *, int, int, int);
+static void xdrawglyphfontspecs(const XftGlyphFontSpec *, Glyph, int, int, int, int);
 static void xdrawglyph(Glyph, int, int);
 static void xhints(void);
 static void xclear(int, int, int, int);
@@ -995,11 +1003,12 @@ getsel(void)
 	char *str, *ptr;
 	int y, bufsize, lastx, linelen;
 	Glyph *gp, *last;
+	Rune *pc;
 
 	if (sel.ob.x == -1)
 		return NULL;
 
-	bufsize = (term.col+1) * (sel.ne.y-sel.nb.y+1) * UTF_SIZ;
+	bufsize = (term.specbuflen+1) * (sel.ne.y-sel.nb.y+1) * UTF_SIZ;
 	ptr = str = xmalloc(bufsize);
 
 	/* append every set & selected glyph to the selection */
@@ -1025,6 +1034,16 @@ getsel(void)
 				continue;
 
 			ptr += utf8encode(gp->u, ptr);
+			if(gp->mode & ATTR_DECOMPPTR) {
+				pc = gp->dc.pc;
+				for(pc = gp->dc.pc; *pc; pc++)
+					ptr += utf8encode(*pc, ptr);
+			} else {
+				if(gp->dc.c[0])
+					ptr += utf8encode(gp->dc.c[0], ptr);
+				if(gp->dc.c[1])
+					ptr += utf8encode(gp->dc.c[1], ptr);
+			}
 		}
 
 		/*
@@ -1861,6 +1880,59 @@ tsetchar(Rune u, Glyph *attr, int x, int y)
 }
 
 void
+taddcchar(Rune u, int x, int y)
+{
+	Rune *p;
+	int sz;
+
+	if(!u)
+		return;
+
+	/* don't add combining chars to wide dummies, add them to the real char */
+	if(term.line[y][x].mode == ATTR_WDUMMY) {
+		if(x == 0) {
+			if(y > 0) {
+				y--;
+				x = term.col-1;
+			} else {
+				/* this shouldn't happen, but if it does... */
+				return;
+			}
+		} else {
+			x--;
+		}
+	}
+	/* can this happen? */
+	if(term.line[y][x].mode == ATTR_WDUMMY)
+		return;
+
+	term.dirty[y] = 1;
+	if(term.line[y][x].mode & ATTR_DECOMPPTR) {
+		p = term.line[y][x].dc.pc;
+		while(*p)
+			p++;
+		sz = (p - term.line[y][x].dc.pc) + 2;
+		term.line[y][x].dc.pc = xrealloc(term.line[y][x].dc.pc, sz * sizeof(Rune));
+		term.line[y][x].dc.pc[sz-2] = u;
+		term.line[y][x].dc.pc[sz-1] = 0;
+	} else {
+		if(!term.line[y][x].dc.c[0]) {
+			term.line[y][x].dc.c[0] = u;
+		} else if(!term.line[y][x].dc.c[1]) {
+			term.line[y][x].dc.c[1] = u;
+		} else {
+			p = xmalloc(4 * sizeof(Rune));
+			p[0] = term.line[y][x].dc.c[0];
+			p[1] = term.line[y][x].dc.c[1];
+			p[2] = u;
+			p[3] = 0;
+			term.line[y][x].dc.pc = p;
+			term.line[y][x].mode |= ATTR_DECOMPPTR;
+		}
+	}
+}
+
+void
 tclearregion(int x1, int y1, int x2, int y2)
 {
 	int x, y, temp;
@@ -1884,7 +1956,11 @@ tclearregion(int x1, int y1, int x2, int y2)
 				selclear(NULL);
 			gp->fg = term.c.attr.fg;
 			gp->bg = term.c.attr.bg;
+			if(gp->mode & ATTR_DECOMPPTR)
+				free(gp->dc.pc);
 			gp->mode = 0;
+			gp->dc.c[0] = 0;
+			gp->dc.c[1] = 0;
 			gp->u = ' ';
 		}
 	}
@@ -1893,7 +1969,7 @@ tclearregion(int x1, int y1, int x2, int y2)
 void
 tdeletechar(int n)
 {
-	int dst, src, size;
+	int dst, src, size, i;
 	Glyph *line;
 
 	LIMIT(n, 0, term.col - term.c.x);
@@ -1903,6 +1979,10 @@ tdeletechar(int n)
 	size = term.col - src;
 	line = term.line[term.c.y];
 
+	for(i = dst; i < src; i++) {
+		if(line[i].mode & ATTR_DECOMPPTR)
+			line[i].dc.pc = NULL;
+	}
 	memmove(&line[dst], &line[src], size * sizeof(Glyph));
 	tclearregion(term.col-n, term.c.y, term.col-1, term.c.y);
 }
@@ -1910,7 +1990,7 @@ tdeletechar(int n)
 void
 tinsertblank(int n)
 {
-	int dst, src, size;
+	int dst, src, size, i;
 	Glyph *line;
 
 	LIMIT(n, 0, term.col - term.c.x);
@@ -1920,6 +2000,10 @@ tinsertblank(int n)
 	size = term.col - dst;
 	line = term.line[term.c.y];
 
+	for(i = src; i < dst; i++) {
+		if(line[i].mode & ATTR_DECOMPPTR)
+			line[i].dc.pc = NULL;
+	}
 	memmove(&line[dst], &line[src], size * sizeof(Glyph));
 	tclearregion(src, term.c.y, dst - 1, term.c.y);
 }
@@ -3011,6 +3095,19 @@ tputc(Rune u)
 	if (sel.ob.x != -1 && BETWEEN(term.c.y, sel.ob.y, sel.oe.y))
 		selclear(NULL);
 
+	/* combining chars are added to the previous cell */
+	if(width == 0) {
+		if(term.c.x == 0) {
+			if(term.c.y == 0)
+				taddcchar(u, 0, 0);
+			else
+				taddcchar(u, term.col-1, term.c.y-1);
+		} else {
+			taddcchar(u, term.c.x-1, term.c.y);
+		}
+		return;
+	}
+
 	gp = &term.line[term.c.y][term.c.x];
 	if (IS_SET(MODE_WRAP) && (term.c.state & CURSOR_WRAPNEXT)) {
 		gp->mode |= ATTR_WRAP;
@@ -3045,7 +3142,7 @@ tputc(Rune u)
 void
 tresize(int col, int row)
 {
-	int i;
+	int i, j;
 	int minrow = MIN(row, term.row);
 	int mincol = MIN(col, term.col);
 	int *bp;
@@ -3063,6 +3160,12 @@ tresize(int col, int row)
 	 * memmove because we're freeing the earlier lines
 	 */
 	for (i = 0; i <= term.c.y - row; i++) {
+		for(j = 0; j < term.col; j++) {
+			if(term.line[i][j].mode & ATTR_DECOMPPTR)
+				free(term.line[i][j].dc.pc);
+			if(term.alt[i][j].mode & ATTR_DECOMPPTR)
+				free(term.alt[i][j].dc.pc);
+		}
 		free(term.line[i]);
 		free(term.alt[i]);
 	}
@@ -3072,12 +3175,19 @@ tresize(int col, int row)
 		memmove(term.alt, term.alt + i, row * sizeof(Line));
 	}
 	for (i += row; i < term.row; i++) {
+		for(j = 0; j < term.col; j++) {
+			if(term.line[i][j].mode & ATTR_DECOMPPTR)
+				free(term.line[i][j].dc.pc);
+			if(term.alt[i][j].mode & ATTR_DECOMPPTR)
+				free(term.alt[i][j].dc.pc);
+		}
 		free(term.line[i]);
 		free(term.alt[i]);
 	}
 
 	/* resize to new width */
-	term.specbuf = xrealloc(term.specbuf, col * sizeof(XftGlyphFontSpec));
+	term.specbuflen = col * 2;
+	term.specbuf = xrealloc(term.specbuf, term.specbuflen * sizeof(XftGlyphFontSpec));
 
 	/* resize to new height */
 	term.line = xrealloc(term.line, row * sizeof(Line));
@@ -3087,14 +3197,26 @@ tresize(int col, int row)
 
 	/* resize each row to new width, zero-pad if needed */
 	for (i = 0; i < minrow; i++) {
+		for(j = col; j < term.col; j++) {
+			if(term.line[i][j].mode & ATTR_DECOMPPTR)
+				free(term.line[i][j].dc.pc);
+			if(term.alt[i][j].mode & ATTR_DECOMPPTR)
+				free(term.alt[i][j].dc.pc);
+		}
 		term.line[i] = xrealloc(term.line[i], col * sizeof(Glyph));
 		term.alt[i]  = xrealloc(term.alt[i],  col * sizeof(Glyph));
+		if(col > term.col) {
+			memset(&term.line[i][term.col], 0, (col - term.col) * sizeof(Glyph));
+			memset(&term.alt[i][term.col], 0, (col - term.col) * sizeof(Glyph));
+		}
 	}
 
 	/* allocate any new rows */
 	for (/* i == minrow */; i < row; i++) {
 		term.line[i] = xmalloc(col * sizeof(Glyph));
 		term.alt[i] = xmalloc(col * sizeof(Glyph));
+		memset(term.line[i], 0, col * sizeof(Glyph));
+		memset(term.alt[i], 0, col * sizeof(Glyph));
 	}
 	if (col > term.col) {
 		bp = term.tabs + term.col;
@@ -3549,12 +3671,13 @@ xinit(void)
 }
 
 int
-xmakeglyphfontspecs(XftGlyphFontSpec *specs, const Glyph *glyphs, int len, int x, int y)
+xmakeglyphfontspecs(XftGlyphFontSpec *specs, int nspecs, const Glyph *glyphs, int len, int x, int y)
 {
-	float winx = borderpx + x * xw.cw, winy = borderpx + y * xw.ch, xp, yp;
+	float winx = borderpx + x * xw.cw, winy = borderpx + y * xw.ch;
 	ushort mode, prevmode = USHRT_MAX;
 	Font *font = &dc.font;
 	int frcflags = FRC_NORMAL;
+	float xp = winx, yp = winy + font->ascent;
 	float runewidth = xw.cw;
 	Rune rune;
 	FT_UInt glyphidx;
@@ -3562,16 +3685,32 @@ xmakeglyphfontspecs(XftGlyphFontSpec *specs, const Glyph *glyphs, int len, int x
 	FcPattern *fcpattern, *fontpattern;
 	FcFontSet *fcsets[] = { NULL };
 	FcCharSet *fccharset;
-	int i, f, numspecs = 0;
+	int i = 0, ci = -1, j, f, numspecs = 0;
 
-	for (i = 0, xp = winx, yp = winy + font->ascent; i < len; ++i) {
+	while(i < len) {
 		/* Fetch rune and mode for current glyph. */
 		rune = glyphs[i].u;
 		mode = glyphs[i].mode;
 
 		/* Skip dummy wide-character spacing. */
-		if (mode == ATTR_WDUMMY)
+		if(mode == ATTR_WDUMMY) {
+			i++;
 			continue;
+		}
+
+		if((j = ci++) >= 0) {
+			if(mode & ATTR_DECOMPPTR)
+				rune = glyphs[i].dc.pc[j];
+			else
+				rune = (j < 2) ? glyphs[i].dc.c[j] : 0;
+
+			if(!rune) {
+				i++;
+				ci = -1;
+				xp += runewidth;
+				continue;
+			}
+		}
 
 		/* Determine font for glyph if different from previous glyph. */
 		if (prevmode != mode) {
@@ -3595,11 +3734,12 @@ xmakeglyphfontspecs(XftGlyphFontSpec *specs, const Glyph *glyphs, int len, int x
 		/* Lookup character index with default font. */
 		glyphidx = XftCharIndex(xw.dpy, font->match, rune);
 		if (glyphidx) {
-			specs[numspecs].font = font->match;
-			specs[numspecs].glyph = glyphidx;
-			specs[numspecs].x = (short)xp;
-			specs[numspecs].y = (short)yp;
-			xp += runewidth;
+			if(numspecs < nspecs) {
+				specs[numspecs].font = font->match;
+				specs[numspecs].glyph = glyphidx;
+				specs[numspecs].x = (short)xp;
+				specs[numspecs].y = (short)yp;
+			}
 			numspecs++;
 			continue;
 		}
@@ -3669,21 +3809,44 @@ xmakeglyphfontspecs(XftGlyphFontSpec *specs, const Glyph *glyphs, int len, int x
 			FcCharSetDestroy(fccharset);
 		}
 
-		specs[numspecs].font = frc[f].font;
-		specs[numspecs].glyph = glyphidx;
-		specs[numspecs].x = (short)xp;
-		specs[numspecs].y = (short)(winy + frc[f].font->ascent);
-		xp += runewidth;
+		if(numspecs < nspecs) {
+			specs[numspecs].font = frc[f].font;
+			specs[numspecs].glyph = glyphidx;
+			specs[numspecs].x = (short)xp;
+			specs[numspecs].y = (short)(winy + frc[f].font->ascent);
+		}
 		numspecs++;
 	}
 
 	return numspecs;
 }
 
+int
+xmakeglyphfontspecsintermbuf(const Glyph *glyphs, int len, int x, int y)
+{
+	int numspecs;
+
+	while(1) {
+		numspecs = xmakeglyphfontspecs(term.specbuf, term.specbuflen, glyphs, len, x, y);
+		if(numspecs <= term.specbuflen)
+			break;
+		if(term.specbuflen == INT_MAX)
+			abort();
+
+		/* combining chars require a larger spec buffer */
+		if(term.specbuflen > INT_MAX/2)
+			term.specbuflen = INT_MAX;
+		else
+			term.specbuflen *= 2;
+		term.specbuf = xrealloc(term.specbuf, term.specbuflen * sizeof(XftGlyphFontSpec));
+	}
+
+	return numspecs;
+}
+
 void
-xdrawglyphfontspecs(const XftGlyphFontSpec *specs, Glyph base, int len, int x, int y)
+xdrawglyphfontspecs(const XftGlyphFontSpec *specs, Glyph base, int len, int charlen, int x, int y)
 {
-	int charlen = len * ((base.mode & ATTR_WIDE) ? 2 : 1);
 	int winx = borderpx + x * xw.cw, winy = borderpx + y * xw.ch,
 	    width = charlen * xw.cw;
 	Color *fg, *bg, *temp, revfg, revbg, truefg, truebg;
@@ -3818,9 +3981,8 @@ void
 xdrawglyph(Glyph g, int x, int y)
 {
 	int numspecs;
-	XftGlyphFontSpec spec;
-	numspecs = xmakeglyphfontspecs(&spec, &g, 1, x, y);
-	xdrawglyphfontspecs(&spec, g, numspecs, x, y);
+	numspecs = xmakeglyphfontspecsintermbuf(&g, 1, x, y);
+	xdrawglyphfontspecs(term.specbuf, g, numspecs, ((g.mode & ATTR_WIDE) ? 2 : 1), x, y);
 }
 
 void
@@ -3828,7 +3990,7 @@ xdrawcursor(void)
 {
 	static int oldx = 0, oldy = 0;
 	int curx;
-	Glyph g = {' ', ATTR_NULL, defaultbg, defaultcs};
+	Glyph g = {' ', {{0, 0}}, ATTR_NULL, defaultbg, defaultcs};
 
 	LIMIT(oldx, 0, term.col-1);
 	LIMIT(oldy, 0, term.row-1);
@@ -3841,7 +4003,9 @@ xdrawcursor(void)
 	if (term.line[term.c.y][curx].mode & ATTR_WDUMMY)
 		curx--;
 
-	g.u = term.line[term.c.y][term.c.x].u;
+	g.u = term.line[term.c.y][curx].u;
+	g.dc = term.line[term.c.y][curx].dc;
+	g.mode |= term.line[term.c.y][curx].mode & ATTR_DECOMPPTR;
 
 	/* remove the old cursor */
 	xdrawglyph(term.line[oldy][oldx], oldx, oldy);
@@ -3940,8 +4104,9 @@ draw(void)
 void
 drawregion(int x1, int y1, int x2, int y2)
 {
-	int i, x, y, ox, numspecs;
+	int i, w, x, y, ox, numspecs;
 	Glyph base, new;
+	Rune *pc;
 	XftGlyphFontSpec* specs;
 	int ena_sel = sel.ob.x != -1 && sel.alt == IS_SET(MODE_ALTSCREEN);
 
@@ -3955,30 +4120,43 @@ drawregion(int x1, int y1, int x2, int y2)
 		xtermclear(0, y, term.col, y);
 		term.dirty[y] = 0;
 
+		numspecs = xmakeglyphfontspecsintermbuf(&term.line[y][x1], x2 - x1, x1, y);
 		specs = term.specbuf;
-		numspecs = xmakeglyphfontspecs(specs, &term.line[y][x1], x2 - x1, x1, y);
 
-		i = ox = 0;
-		for (x = x1; x < x2 && i < numspecs; x++) {
+		i = w = 0;
+		for(ox = x = x1; x < x2 && i < numspecs; x++) {
 			new = term.line[y][x];
 			if (new.mode == ATTR_WDUMMY)
 				continue;
 			if (ena_sel && selected(x, y))
 				new.mode ^= ATTR_REVERSE;
 			if (i > 0 && ATTRCMP(base, new)) {
-				xdrawglyphfontspecs(specs, base, i, ox, y);
+				xdrawglyphfontspecs(specs, base, i, w, ox, y);
 				specs += i;
 				numspecs -= i;
 				i = 0;
 			}
 			if (i == 0) {
-				ox = x;
+				ox += w;
 				base = new;
+				w = 0;
+			}
+			if(new.mode & ATTR_DECOMPPTR) {
+				pc = new.dc.pc;
+				while(*pc)
+					pc++;
+				i += pc - new.dc.pc;
+			} else {
+				if(new.dc.c[0])
+					i++;
+				if(new.dc.c[1])
+					i++;
 			}
 			i++;
+			w += (new.mode & ATTR_WIDE) ? 2 : 1;
 		}
 		if (i > 0)
-			xdrawglyphfontspecs(specs, base, i, ox, y);
+			xdrawglyphfontspecs(specs, base, i, w, ox, y);
 	}
 	xdrawcursor();
 }

Reply via email to