9front - general discussion about 9front
 help / color / mirror / Atom feed
* truetypefs
@ 2020-10-30 17:45 nicolagi
  2020-10-31 11:42 ` [9front] truetypefs cinap_lenrek
  2020-11-06 16:07 ` ori
  0 siblings, 2 replies; 3+ messages in thread
From: nicolagi @ 2020-10-30 17:45 UTC (permalink / raw)
  To: 9front

Hi all,

truetypefs used to crash for me; it used to happen when trying to display
some non-ASCII glyph, e.g., an accented vowel: à.

Commenting out the assertion at /sys/src/libttf/scan.c:371 solves the
problem for me, although I fail to grasp why, and whether it's the
right fix; I'll share why I gleaned so far.

(Since text renders correctly on my end, I presume the assertion is
wrong, but it's really just a guess.)

The flags field of TTPoint defined at
/sys/include/ttf.h:/^struct.TTPoint/ seems to hold values defined at
/sys/src/libttf/impl.h:/ONCURVE/.  So in other words, the assertion I
commented out fails because a point is expected to be on the curve,
while it is not.  What point is it?  If I were to guess from
/sys/src/libttf/scan.c:370, I'd say the final point of each countour.

AFAICS, the only place that can clear the bit is
/sys/src/libttf/hint.c:1377, part of some hinting operation (flip
range).  Is the operation wrong, or is the assertion wrong?

I'm including a patch that replaces magic numbers with enum values and
comments out the problematic assertion.  I realize it's not of much
value but I hope someone can improve on it.

Thanks,

Nicola

diff -r f4aa654bc414 sys/include/ttf.h
--- a/sys/include/ttf.h	Thu Oct 29 18:26:35 2020 +0100
+++ b/sys/include/ttf.h	Fri Oct 30 17:33:50 2020 +0000
@@ -31,6 +31,7 @@
 };
 struct TTPoint {
 	int x, y;
+	/* ONCURVE, TOUCHX, TOUCHY, TOUCH. */
 	u8int flags;
 };
 struct TTBitmap {
diff -r f4aa654bc414 sys/src/libttf/glyf.c
--- a/sys/src/libttf/glyf.c	Thu Oct 29 18:26:35 2020 +0100
+++ b/sys/src/libttf/glyf.c	Fri Oct 30 17:33:50 2020 +0000
@@ -135,7 +135,7 @@
 		goto err;
 	for(i = 0; i < np; i++){
 		p = &g->pt[g->npt + i];
-		p->flags = *fp & 1;
+		p->flags = *fp & ONCURVE;
 		switch(*fp++ & 0x12){
 		case 0x00: ttfunpack(f, "w", &temp16); p->x = lastx += temp16; break;
 		case 0x02: ttfunpack(f, "b", &temp8); p->x = lastx -= temp8; break;
diff -r f4aa654bc414 sys/src/libttf/hint.c
--- a/sys/src/libttf/hint.c	Thu Oct 29 18:26:35 2020 +0100
+++ b/sys/src/libttf/hint.c	Fri Oct 30 17:33:50 2020 +0000
@@ -136,8 +136,8 @@
 	if((n & RP2) != 0)
 		pi = h->f->rp[(n >> 4 & 3) - 1];
 	if((n & NOTOUCH) == 0){
-		if(h->f->fvx != 0) p.flags |= 2;
-		if(h->f->fvy != 0) p.flags |= 4;
+		if(h->f->fvx != 0) p.flags |= TOUCHX;
+		if(h->f->fvy != 0) p.flags |= TOUCHY;
 	}
 	if((h->f->zp >> (n >> 8 & 3) & 1) != 0){
 		if(h->g == nil)
@@ -1374,7 +1374,7 @@
 		herror(h, "FLIPRG without glyph");
 	for(; i <= e; i++)
 		if((int)i < h->g->npt)
-			h->g->pt[i].flags = h->g->pt[i].flags & ~1 | h->ip[-1] & 1;
+			h->g->pt[i].flags = h->g->pt[i].flags & ~ONCURVE | h->ip[-1] & 1;
 }
 
 static void
diff -r f4aa654bc414 sys/src/libttf/scan.c
--- a/sys/src/libttf/scan.c	Thu Oct 29 18:26:35 2020 +0100
+++ b/sys/src/libttf/scan.c	Fri Oct 30 17:33:50 2020 +0000
@@ -368,9 +368,9 @@
 	for(i = 0; i < g->ncon; i++){
 		if(g->confst[i] + 1 >= g->confst[i+1]) continue;
 		p = g->pt[g->confst[i]];
-		assert((p.flags & 1) != 0);
+		/* assert((p.flags & ONCURVE) != 0); */
 		for(j = g->confst[i]; j++ < g->confst[i+1]; ){
-			if(j < g->confst[i+1] && (g->pt[j].flags & 1) == 0)
+			if(j < g->confst[i+1] && (g->pt[j].flags & ONCURVE) == 0)
 				q = g->pt[j++];
 			else
 				q = p;
@@ -378,14 +378,14 @@
 				r = g->pt[g->confst[i]];
 			else{
 				r = g->pt[j];
-				if((g->pt[j].flags & 1) == 0){
+				if((g->pt[j].flags & ONCURVE) == 0){
 					r.x = (r.x + q.x) / 2;
 					r.y = (r.y + q.y) / 2;
 				}
 			}
 			dobezier(&s, p, q, r);
 			p = r;
-			if(j < g->confst[i+1] && (g->pt[j].flags & 1) == 0)
+			if(j < g->confst[i+1] && (g->pt[j].flags & ONCURVE) == 0)
 				j--;
 		}
 	}
@@ -440,9 +440,9 @@
 		(*fp)[0] = p.x * scale;
 		(*fp)[1] = p.y * scale + offy;
 	}
-	assert((p.flags & 1) != 0);
+	assert((p.flags & ONCURVE) != 0);
 	for(j = g->confst[i]; j++ < g->confst[i+1]; ){
-		if(j < g->confst[i+1] && (g->pt[j].flags & 1) == 0)
+		if(j < g->confst[i+1] && (g->pt[j].flags & ONCURVE) == 0)
 			q = g->pt[j++];
 		else
 			q = p;
@@ -450,7 +450,7 @@
 			r = g->pt[g->confst[i]];
 		else{
 			r = g->pt[j];
-			if((g->pt[j].flags & 1) == 0){
+			if((g->pt[j].flags & ONCURVE) == 0){
 				r.x = (r.x + q.x) / 2;
 				r.y = (r.y + q.y) / 2;
 			}
@@ -469,7 +469,7 @@
 		}
 		p = r;
 		n += 2;
-		if(j < g->confst[i+1] && (g->pt[j].flags & 1) == 0)
+		if(j < g->confst[i+1] && (g->pt[j].flags & ONCURVE) == 0)
 			j--;
 	}
 	if(np != nil)



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [9front] truetypefs
  2020-10-30 17:45 truetypefs nicolagi
@ 2020-10-31 11:42 ` cinap_lenrek
  2020-11-06 16:07 ` ori
  1 sibling, 0 replies; 3+ messages in thread
From: cinap_lenrek @ 2020-10-31 11:42 UTC (permalink / raw)
  To: 9front

thanks!

it looks fine to me.

i forwarded the patch to the original author... she's the
expert on this and might explain why that assert was there.

--
cinap


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [9front] truetypefs
  2020-10-30 17:45 truetypefs nicolagi
  2020-10-31 11:42 ` [9front] truetypefs cinap_lenrek
@ 2020-11-06 16:07 ` ori
  1 sibling, 0 replies; 3+ messages in thread
From: ori @ 2020-11-06 16:07 UTC (permalink / raw)
  To: 9front

> Hi all,
> 
> truetypefs used to crash for me; it used to happen when trying to display
> some non-ASCII glyph, e.g., an accented vowel: à.

So, I remember looking into this a while back, and I asked aiju
about it at the time -- here's the response:

 aiju  | Ori_B: there is a dumb corner case i didn't expect any font to hit
 aiju  | but some fonts hit it anyway
 Ori_B | oh?
 aiju  | Ori_B: where ALL the points on a curve are off-curve
 aiju  | this might be the same bug
 Ori_B | oh... hm.
 aiju  | it's probably not too hard to figure out but i have all this shit paged out right now
 Ori_B | I can verify that at least.
 aiju  | i think you need to pick two points and calculate the average and use that as a starting point
 Ori_B | that sounds reasonable.
 aiju  | i literally just didn't implement it because i figured no font would do this shit
 Ori_B | yeah, it's some unicode junk that I appear to have accidentally gotten into the
         file, so I didn't hit it on a common glyph.
 Ori_B | should we also return a 'bogus glyph' instead of killing font rendering?
 Ori_B | '?', pjw face, something?



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-11-06 16:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-30 17:45 truetypefs nicolagi
2020-10-31 11:42 ` [9front] truetypefs cinap_lenrek
2020-11-06 16:07 ` ori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).