Bug 689559

Summary: centerline of vertical CJK text is wrong when external TrueType CJK font is used.
Product: Ghostscript Reporter: mpsuzuki <mpsuzuki>
Component: PS InterpreterAssignee: leonardo <leonardo>
Status: NOTIFIED FIXED    
Severity: major CC: leonardo
Priority: P2    
Version: master   
Hardware: PC   
OS: All   
Customer: Word Size: ---
Bug Depends on:    
Bug Blocks: 689304    
Attachments: writing vertical text on vertical blue line
rendering result by HEAD
part of Koji Otani patch to fix this issue
fixed result by the part of Koji Otani's patch
Revised Koji Otani's patch
Re-Revised Koji Otani's patch
patch to fix vertical metrics utlization in zchar42_set_cache()
patch.txt

Description mpsuzuki 2007-11-13 07:22:29 UTC
It is one of the bugs which Koji Otani's patch for bug 689304.
In vertical writing mode, the center of each glyphs should be
aligned to single vertical line. It is rendered correctly by
ghostscript 7.07, and final ESP ghostscript including CJK patch.
HEAD align the left side of each glyphs to a vertical line.
Comment 1 mpsuzuki 2007-11-13 07:24:48 UTC
Created attachment 3552 [details]
writing vertical text on vertical blue line

Sample file
You must setup your cidfont to provide
Ryumin-Light CIDFont by some CJK TrueType font.
Comment 2 mpsuzuki 2007-11-13 07:28:36 UTC
Created attachment 3553 [details]
rendering result by HEAD
Comment 3 mpsuzuki 2007-11-13 07:31:28 UTC
Created attachment 3554 [details]
part of Koji Otani patch to fix this issue

part of Koji Otani's patch submitted to bug 689304,
it fixes this issue.
Comment 4 mpsuzuki 2007-11-13 07:32:54 UTC
Created attachment 3555 [details]
fixed result by the part of Koji Otani's patch
Comment 5 mpsuzuki 2007-11-13 07:33:45 UTC
Now under regression test.
Comment 6 mpsuzuki 2007-11-13 13:47:16 UTC
no regression is found.
Comment 7 mpsuzuki 2007-11-14 08:06:30 UTC
Created attachment 3561 [details]
Revised Koji Otani's patch

3 trivial remarks are revised.

* gs_glyph_data_free() should be called with client function name.
* implicit typecast in recursive calling of gs_type42_glyph_fbbox()
* if notdef glyph is loaded, 

--

Fix: Wrong centerline of vertical text drawn by CJK TrueType fonts.

DETAILS:
In vertical writing mode, when zchar_get_metrics() returns
successfully but returned value is "metricsNone", the vertical
metric obtained by gs_type42_default_get_metrics() with a flag
gs_type42_metrics_options_WMODE1_AND_BBOX is not reliable.

The metric for vertical writing mode is synthesized from
per-glyph bounding box (pbbox) obtained by new function
gs_type42_glyph_fbbox() and horizontal metric obtained by
gs_type42_default_get_metrics() with a flag
gs_type42_metrics_options_WMODE0_AND_BBOX. Fixes bug 689559.

"pbbox" is filled by calling gs_type42_glyph_fbbox() which
parses the entry of glyf table for given glyph index and
updates both of per-glyph and per-font bounding box.
If per-glyph bounding box is unavailable, per-font bounding
box is used.

EXPECTED DIFFERENCES:
None.
Comment 8 mpsuzuki 2007-11-15 19:28:24 UTC
Created attachment 3566 [details]
Re-Revised Koji Otani's patch

Revised to skip calling gs_type42_glyph_fbbox() when writing
mode is not vertical. This function is designed to fix the
bug of metrics in vertical writing mode, calling it always
is unnecessary.
Comment 9 leonardo 2007-11-22 12:50:51 UTC
Reviewing the patch 3566 :

[beg quote]
+ * Modified by AXE,Inc., BBR Inc. and Turbolinux Inc.
+ *   under the technical advice by suzuki toshiya (Hiroshima University)
+ * For questions, please send mail to espgs8-cjk@printing-japan.org
+ *
+ * (C) Copyright 2006 Center of the International Cooperation for
+ *     Computerization
[end quote]

This copyright is not acceptable for Artifex releases. Please either submit 
another patch or negotiate with the mentioned enterprises about licensing their 
patch to Artifex. Thanks for understannding.
Comment 10 leonardo 2007-11-22 23:27:13 UTC
From the log message :

[beg quote]
"pbbox" is filled by calling gs_type42_glyph_fbbox() which
parses the entry of glyf table for given glyph index and
updates both of per-glyph and per-font bounding box.
[end quote]

This solution must not go to production, because the glyph position depends on 
what glyphs are printed before it ("before" in the CPU time, which may be 
different than the document's text order). Same character may get different 
offsets if another character appears between 2 occurances of same character, as 
in the word "init". The effect will be stablely visible with -dNOCACHE.
Comment 11 mpsuzuki 2007-12-20 00:36:59 UTC
This bug was introduced by SVN revision 7567.
SVN revision 7566 was the final revision that can render
the glyphs on correct centerline. I think Otani's patch
was trying to restore original correct behaviour.

>The effect will be stablely visible with -dNOCACHE.

Hmm. According to the log message, SVN revision 7567
changes the metrics handling in glyph cache:
zchar42_set_cache() is changed, although the motivation
of the changeset come not from glyph cache but from
wrong glyph metrics. Why the changeset 7567 modified
zchar42_set_cache()?
Comment 12 leonardo 2007-12-20 01:19:00 UTC
Regarding Comment #11: I afraid now I can't add anything to the rev 7567 log. 
It was done for CET complience. Sorry now I can't recall details becauase I 
worked on many other issues after that.

I still think that Otani's patch should not go. A better idea is to unite all 
glyph boxes when the font is loaded. But we don't want to scan all ideograms 
which all have a "standard" bbox, so only a small part of a character 
collection to be scanned. I think CJK specialist can define the part better 
than me. In any case we don't want a sesnible slowdown.

By another hand, likely the font is incorrect, so I'm in doubt whether we must 
handle it at all. CET conformity is preferrable.


Comment 13 leonardo 2007-12-20 01:20:36 UTC
Please also pay attention to the change to trunk/gs/doc/pscet_status.txt in rev 
7567. The removed text explains the reason.
Comment 14 mpsuzuki 2008-02-18 00:34:11 UTC
Created attachment 3794 [details]
patch to fix vertical metrics utlization in zchar42_set_cache()

For the bug since SVN revision 7567, the problematic part
in zchar42.c::zchar42_set_cache() is following.

-----------------------------------------------------------------

When vertical metrics is found and we setup sbw[] array 

SVN 7566: sbw[] is horizontal, sbw42[] is vertical
	sbw[0] = sbw[2] / 2;
	sbw[1] = (pbfont->FontBBox.q.y + pbfont->FontBBox.p.y - sbw42[3]) / 2;
	sbw[2] = sbw42[2];
	sbw[3] = sbw42[3];
	present = metricsSideBearingAndWidth;
SVN 7567: sbw[]_bbox is vertical
	sbw[0] = sbw_bbox[2] / 2;
	sbw[1] = (pbfont->FontBBox.q.y + pbfont->FontBBox.p.y - sbw_bbox[3]) /
2;
	sbw[2] = sbw_bbox[2];
	sbw[3] = sbw_bbox[3];
	present = metricsSideBearingAndWidth;

-----------------------------------------------------------------

See the part calculating sbw[0] when vertical metrics is
available. In SVN revision 7566, sbw[0] is always calculated
from horizontal metrics. Since 7567, if vertical metrics is
found, sbw[0] is calculated from it. If vertical sbw[0] is
calculated from horizontal metrics always, the bug disappears.
Attached is the patch to do that.

In the patch (and SVN revision 7566), always horizontal metrics
is used. The utilization of sbw[] and sbw_bbox[] in SVN revision
7566 zchar42.c is too complicated, I introduced explicitly-named
variables hsbw_bbox[], vsbw_bbox[] (storage for metric/bbox infos
for each writing modes) and hcode, vcode (metrics availability for
each writing modes) for easier maintenance.

Index: src/zchar42.c
===================================================================
--- src/zchar42.c	(revision 8527)
+++ src/zchar42.c	(working copy)
@@ -40,76 +40,71 @@
	    uint glyph_index, op_proc_t cont, op_proc_t *exec_cont, bool
put_lsb)
 {   double sbw[4];
     double w[2];
-    int present;
+    int present = zchar_get_metrics(pbfont, cnref, sbw);
     gs_font_type42 *pfont42 = (gs_font_type42 *)pbfont;
-    int code = zchar_get_metrics(pbfont, cnref, sbw);
     gs_rect bbox;
     int vertical = gs_rootfont(igs)->WMode;
-    float sbw_bbox[8];
+    int hcode, vcode;
+    float hsbw_bbox[8], vsbw_bbox[8];
 
-    if (code < 0)
-	return code;
-    present = code;
+    /* horizontal metrics is always required */
+    hcode = pfont42->data.get_metrics(pfont42, glyph_index, 
+	      gs_type42_metrics_options_WMODE0_AND_BBOX, hsbw_bbox);
+    if (hcode < 0)
+	 return hcode;
+
     if (vertical) { /* for vertically-oriented metrics */
-	code = pfont42->data.get_metrics(pfont42, glyph_index, 
-		gs_type42_metrics_options_WMODE1_AND_BBOX, sbw_bbox);
-	if (code < 0) { 
+	vcode = pfont42->data.get_metrics(pfont42, glyph_index, 
+		gs_type42_metrics_options_WMODE1_AND_BBOX, vsbw_bbox);
+	if (vcode < 0) { 
	    /* No vertical metrics in the font, compose from horizontal. 
	       Still need bbox also. */
-	    code = pfont42->data.get_metrics(pfont42, glyph_index, 
-		    gs_type42_metrics_options_WMODE0_AND_BBOX, sbw_bbox);
-	    if (code < 0)
-		return code;
	    if (present == metricsNone) {
		if (pbfont->FontType == ft_CID_TrueType) {
-		    sbw[0] = sbw_bbox[2] / 2;
+		    sbw[0] = hsbw_bbox[2] / 2;
		    sbw[1] = pbfont->FontBBox.q.y;
		    sbw[2] = 0;
		    sbw[3] = pbfont->FontBBox.p.y - pbfont->FontBBox.q.y;
		} else {
-		    sbw[0] = sbw_bbox[0];
-		    sbw[1] = sbw_bbox[1];
-		    sbw[2] = sbw_bbox[2];
-		    sbw[3] = sbw_bbox[3];
+		    sbw[0] = hsbw_bbox[0];
+		    sbw[1] = hsbw_bbox[1];
+		    sbw[2] = hsbw_bbox[2];
+		    sbw[3] = hsbw_bbox[3];
		}
		present = metricsSideBearingAndWidth;
	    }
	} else {
	    if (present == metricsNone) {
-		sbw[0] = sbw_bbox[2] / 2;
-		sbw[1] = (pbfont->FontBBox.q.y + pbfont->FontBBox.p.y -
sbw_bbox[3]) / 2;
-		sbw[2] = sbw_bbox[2];
-		sbw[3] = sbw_bbox[3];
+		sbw[0] = hsbw_bbox[2] / 2;
+		sbw[1] = (pbfont->FontBBox.q.y + pbfont->FontBBox.p.y -
vsbw_bbox[3]) / 2;
+		sbw[2] = vsbw_bbox[2];
+		sbw[3] = vsbw_bbox[3];
		present = metricsSideBearingAndWidth;
	    }
	}
     } else {
-	code = pfont42->data.get_metrics(pfont42, glyph_index, 
-		    gs_type42_metrics_options_WMODE0_AND_BBOX, sbw_bbox);
-	if (code < 0)
-	    return code;
	if (present == metricsNone) {
-	    sbw[0] = sbw_bbox[0];
-	    sbw[1] = sbw_bbox[1];
-	    sbw[2] = sbw_bbox[2];
-	    sbw[3] = sbw_bbox[3];
+	    sbw[0] = hsbw_bbox[0];
+	    sbw[1] = hsbw_bbox[1];
+	    sbw[2] = hsbw_bbox[2];
+	    sbw[3] = hsbw_bbox[3];
	    present = metricsSideBearingAndWidth;
	}
     }
     w[0] = sbw[2];
     w[1] = sbw[3];
     if (!vertical) {
-	sbw_bbox[6] = (sbw_bbox[6] - sbw_bbox[4]) + sbw_bbox[0];
-	sbw_bbox[4] = sbw_bbox[0];
+	hsbw_bbox[6] = (hsbw_bbox[6] - hsbw_bbox[4]) + hsbw_bbox[0];
+	hsbw_bbox[4] = hsbw_bbox[0];
     }
     /* Note: The glyph bbox usn't useful for Dynalab fonts,
	which stretch subglyphs. Uniting with FontBBox helps.
	In same time, FontBBox with no glyph bbox
	doesn't work for 34_all.PS page 4. */
-    bbox.p.x = min(sbw_bbox[4], pbfont->FontBBox.p.y);
-    bbox.p.y = min(sbw_bbox[5], pbfont->FontBBox.p.y);
-    bbox.q.x = max(sbw_bbox[6], pbfont->FontBBox.q.x);
-    bbox.q.y = max(sbw_bbox[7], pbfont->FontBBox.q.y);
+    bbox.p.x = min(vertical ? vsbw_bbox[4] : hsbw_bbox[4],
pbfont->FontBBox.p.y);
+    bbox.p.y = min(vertical ? vsbw_bbox[5] : hsbw_bbox[5],
pbfont->FontBBox.p.y);
+    bbox.q.x = max(vertical ? vsbw_bbox[6] : hsbw_bbox[6],
pbfont->FontBBox.q.x);
+    bbox.q.y = max(vertical ? vsbw_bbox[7] : hsbw_bbox[7],
pbfont->FontBBox.q.y);
     return zchar_set_cache(i_ctx_p, pbfont, cnref,
			   (put_lsb && present == metricsSideBearingAndWidth ?
			    sbw : NULL),
Comment 15 leonardo 2008-05-21 07:41:15 UTC
Changing assignment for checking the patch.
Comment 16 leonardo 2008-08-10 14:08:43 UTC
Created attachment 4268 [details]
patch.txt

A suggested patch for this bug and for 688058. It is being tested now. Its
commitment to be postponed until gs/src reorganization.
Comment 17 leonardo 2008-08-10 15:17:46 UTC
The patch 4268 passed local test. he comment #14 patch must not go due to 
overcomplicated.
Comment 18 leonardo 2008-08-12 00:44:51 UTC
Patch to HEAD :
http://ghostscript.com/pipermail/gs-cvs/2008-August/008552.html