After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 664533 - Support basic vertical writing
Support basic vertical writing
Status: RESOLVED OBSOLETE
Product: librsvg
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: librsvg maintainers
librsvg maintainers
Depends on:
Blocks:
 
 
Reported: 2011-11-22 05:24 UTC by Kurosawa Takeshi
Modified: 2017-12-13 17:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test of vertical writing text (3.03 KB, image/svg+xml)
2011-11-22 05:24 UTC, Kurosawa Takeshi
  Details
Patch rv. 1.0 (19.10 KB, patch)
2011-11-22 05:31 UTC, Kurosawa Takeshi
none Details | Review
Rendering result with patch (90.43 KB, image/png)
2011-11-22 05:36 UTC, Kurosawa Takeshi
  Details
[PATCH 1/3] Set correct value when unicode-bidi: inherit; (894 bytes, patch)
2011-11-25 15:37 UTC, Kurosawa Takeshi
committed Details | Review
[PATCH 2/3] Remove unused member 'orientation' (1.00 KB, patch)
2011-11-25 15:40 UTC, Kurosawa Takeshi
committed Details | Review
[PATCH 3/3] Support basic vertical writing (Gnome Bug #664533) (17.05 KB, patch)
2011-11-25 16:01 UTC, Kurosawa Takeshi
committed Details | Review
test rendering (84.88 KB, image/png)
2011-11-26 17:10 UTC, Christian Persch
  Details
Modified version of text-intro-03-b (670 bytes, image/svg+xml)
2011-11-28 06:20 UTC, Kurosawa Takeshi
  Details
Modified version of text-intro-03-b (4.83 KB, image/svg+xml)
2011-11-28 06:23 UTC, Kurosawa Takeshi
  Details
Rendering result of modified text-intro-03-b (23.25 KB, image/png)
2011-11-28 06:29 UTC, Kurosawa Takeshi
  Details
Partial support of glyph-orientation-{vertical, horizontal} (12.33 KB, patch)
2011-11-29 07:21 UTC, Kurosawa Takeshi
none Details | Review
Testcase for glyph-orientation-{vertical, horizontal} (38.32 KB, image/png)
2011-11-29 07:43 UTC, Kurosawa Takeshi
  Details
Rendering result with patch (38.32 KB, image/png)
2011-11-29 07:46 UTC, Kurosawa Takeshi
  Details
Rendering result from Chromium browser (correct) (68.27 KB, image/png)
2011-11-29 07:52 UTC, Kurosawa Takeshi
  Details
Updated patch for glyph-orientation-{vertical, horizontal} (19.57 KB, patch)
2012-09-09 15:05 UTC, Kurosawa Takeshi
none Details | Review

Description Kurosawa Takeshi 2011-11-22 05:24:14 UTC
Created attachment 201896 [details]
Test of vertical writing text

Pango has supported basic vertical writing since 1.16.0.
http://mail.gnome.org/archives/gnome-announce-list/2007-February/msg00070.html
http://developer.gnome.org/pango/stable/pango-Vertical-Text.html

Therefore we could support basic vertical writing of SVG text.
http://www.w3.org/TR/SVG/text.html#SettingInlineProgressionDirection


# Although we could not implement all combinations of writing-mode,
# glyph-orientation-horizontal and glyph-orientation-vertical
# (because of Pango API's limitation), 
# it is possible to implement some combinations by using current API.

writing-mode: lr;
  * glyph-orientation-horizontal: 0;    (default)
        PANGO_GRAVITY_SOUTH & PANGO_GRAVITY_HINT_STRONG

  * glyph-orientation-horizontal: 210;
        PANGO_GRAVITY_EAST & PANGO_GRAVITY_HINT_STRONG

writing-mode: tb;
  * glyph-orientation-vertical: auto;   (default)
        PANGO_GRAVITY_EAST & PANGO_GRAVITY_HINT_NATURAL

  * glyph-orientation-vertical: 0;
        PANGO_GRAVITY_EAST & PANGO_GRAVITY_HINT_STRONG

  * glyph-orientation-vertical: 90;
        PANGO_GRAVITY_SOUTH & PANGO_GRAVITY_HINT_STRONG
Comment 1 Kurosawa Takeshi 2011-11-22 05:31:57 UTC
Created attachment 201897 [details] [review]
Patch rv. 1.0

Basic support of vertical writing.
This patch implements writing-mode: tb|tb-rl;
# (no glyph-orientation-vertical and glyph-orientation-horizontal)

This patch also improves rendering of x, y, dx, dy attribute of text/tspan in both vertical and horizontal texts.
Comment 2 Kurosawa Takeshi 2011-11-22 05:36:40 UTC
Created attachment 201898 [details]
Rendering result with patch
Comment 3 Christian Persch 2011-11-24 17:24:08 UTC
+    if (!PANGO_GRAVITY_IS_VERTICAL(gravity)) {
[...]
+    } else {

I think it would be more natural to just invert the order here, so:
if (PANGO_GRAVITY_IS_VERTICAL(...)) {...} else {...}. (Same in all places this occurs.)

+        cairo_rotate (render->cr, -rotation);

Maybe do this conditionally: if (rotation != 0.) ?

     if (state->fill) {
         cairo_move_to (render->cr, x, y);
         rsvg_bbox_insert (&render->bbox, &bbox);
         _set_source_rsvg_paint_server (ctx,
                                        state->current_color,
                                        state->fill,
                                        state->fill_opacity,
                                        bbox, rsvg_current_state (ctx)->current_color);
 
+        cairo_rotate (render->cr, -rotation);
         pango_cairo_show_layout (render->cr, layout);
     }
 
     if (state->stroke) {
         cairo_move_to (render->cr, x, y);
         rsvg_bbox_insert (&render->bbox, &bbox);
+        cairo_save (render->cr);
+        cairo_rotate (render->cr, -rotation);
         pango_cairo_layout_path (render->cr, layout);
+        cairo_restore (render->cr);

This doesn't look right... if it's fill & stroke, won't this rotate the stroke twice? I think both the fill and the stroke case need to be surrounded by cairo_save/restore. (And do that as first/last thing in the block.)

I think the code in rsvg_parse_style_pair() should handle "lr" value explicitly instead of just in the final catch-all else {...}.

Also, can you please attach patches in git format-patch format? That makes applying them easier. Thanks!
Comment 4 Kurosawa Takeshi 2011-11-25 15:37:26 UTC
Created attachment 202135 [details] [review]
[PATCH 1/3] Set correct value when unicode-bidi: inherit;

Thank you for your review!

In fact, this patch is not required to support vertical writing.
However I noticed that misusage.
Comment 5 Kurosawa Takeshi 2011-11-25 15:40:51 UTC
Created attachment 202136 [details] [review]
[PATCH 2/3] Remove unused member 'orientation'

RsvgTextLayout's 'orientation' is never used.
Comment 6 Kurosawa Takeshi 2011-11-25 16:01:38 UTC
Created attachment 202141 [details] [review]
[PATCH 3/3] Support basic vertical writing (Gnome Bug #664533)

This is main patch.

(In reply to comment #3)
> +    if (!PANGO_GRAVITY_IS_VERTICAL(gravity)) {
> [...]
> +    } else {
> 
> I think it would be more natural to just invert the order here, so:
> if (PANGO_GRAVITY_IS_VERTICAL(...)) {...} else {...}. (Same in all places this
> occurs.)

Addressed.

 
> +        cairo_rotate (render->cr, -rotation);
> 
> Maybe do this conditionally: if (rotation != 0.) ?

Addressed.


>      if (state->fill) {
>          cairo_move_to (render->cr, x, y);
>          rsvg_bbox_insert (&render->bbox, &bbox);
>          _set_source_rsvg_paint_server (ctx,
>                                         state->current_color,
>                                         state->fill,
>                                         state->fill_opacity,
>                                         bbox, rsvg_current_state
> (ctx)->current_color);
> 
> +        cairo_rotate (render->cr, -rotation);
>          pango_cairo_show_layout (render->cr, layout);
>      }
> 
>      if (state->stroke) {
>          cairo_move_to (render->cr, x, y);
>          rsvg_bbox_insert (&render->bbox, &bbox);
> +        cairo_save (render->cr);
> +        cairo_rotate (render->cr, -rotation);
>          pango_cairo_layout_path (render->cr, layout);
> +        cairo_restore (render->cr);
> 
> This doesn't look right... if it's fill & stroke, won't this rotate the stroke
> twice? I think both the fill and the stroke case need to be surrounded by
> cairo_save/restore. (And do that as first/last thing in the block.)
> 

Yes. this caused a misrendering and is address.


> I think the code in rsvg_parse_style_pair() should handle "lr" value explicitly
> instead of just in the final catch-all else {...}.
> 

Parsing writing-mode is simplified because lr, rl, and tb are shorthand of lr-tb, rl-tb,and tb-rl in SVG 1.1.
# SVG Tiny 1.2 doesn't support writing-mode (and vertical writing).
Additionally, PANGO_DIRECTION_TTB_XXX are deprecated.
http://developer.gnome.org/pango/stable/pango-Bidirectional-Text.html#PangoDirection
Comment 7 Christian Persch 2011-11-25 21:44:46 UTC
Patch 1/3 and 2/3 look fine; I've pushed them. (Do you have gnome git commit rights, btw?)

Using patch 3/3, the rendering of the attached testcase (attachment 201896 [details]) doesn't match with the rendering in attachment 201898 [details] however...
Comment 8 Kurosawa Takeshi 2011-11-26 14:05:16 UTC
(In reply to comment #7)
> Patch 1 [details]/3 and 2/3 look fine; I've pushed them. (Do you have gnome git commit
> rights, btw?)

Thanks. And I don't have the commit rights.

> Using patch 3 [details]/3, the rendering of the attached testcase (attachment 201896 [details])
> doesn't match with the rendering in attachment 201898 [details] however...

Hmm, it works fine for me. Could you attach the rendering result?
My environment is following:
* Ubuntu 11.10
* cairo 1.10.2
* pango 1.29.3
* gtk 3.2.0/gtk 2.24.6

It could be pango fails rendering vertical texts if no suitable fonts are installed (I'm not confident though).
In my environment,  several Japanese fonts including
IPA font (http://ossipedia.ipa.go.jp/ipafont/index.html) and
M+ font (http://mplus-fonts.sourceforge.jp/mplus-outline-fonts/index-en.html)
are installed.
Comment 9 Christian Persch 2011-11-26 17:10:54 UTC
Created attachment 202194 [details]
test rendering

The final dots are placed differently than in attachment 201898 [details], and I wasn't sure which rendering is right.
Comment 10 Christian Persch 2011-11-26 17:21:20 UTC
Also the rendering of http://www.w3.org/Graphics/SVG/Test/20110816/harness/htmlObjectApproved/text-intro-03-b.html isn't right.
Comment 11 Kurosawa Takeshi 2011-11-28 06:20:15 UTC
Created attachment 202271 [details]
Modified version of text-intro-03-b

(In reply to comment #9)
> Created an attachment (id=202194) [details]
> test rendering
> 
> The final dots are placed differently than in attachment 201898 [details], and I wasn't
> sure which rendering is right.

Some characters (including dots) have two glyphs.
One is for horizontal writing. And the other is for vertical.
http://en.wikipedia.org/wiki/File:Tateyoko.png

However some fonts don't have glyphs for vertical writing.
Thus dots placement can vary.


(In reply to comment #10)
> Also the rendering of
> http://www.w3.org/Graphics/SVG/Test/20110816/harness/htmlObjectApproved/text-intro-03-b.html
> isn't right.

The test case doesn't include language information (i.e. xml:lang attributes).
So font substitution and rendering can fail if pango fails to detect languages.  
I attached modified version of test case which has xml:lang attributes for each text elements.
This would not produce mis-rendering.

The right two sentences "Se: 私は"/"Japanese: 私は..." use glyph-orientation-vertical property.
Since the patch doesn't support that property, these are rendered likely
center two sentences "Se: 私は"/"Japanese: 私は...".
Comment 12 Kurosawa Takeshi 2011-11-28 06:23:13 UTC
Created attachment 202272 [details]
Modified version of text-intro-03-b

Oops! I attached wrong file. This is correct.
Comment 13 Kurosawa Takeshi 2011-11-28 06:29:43 UTC
Created attachment 202273 [details]
Rendering result of modified text-intro-03-b

The rendering result of attachment 202272 [details] with patch.
Comment 14 Christian Persch 2011-11-28 12:50:28 UTC
Thanks for the explanations! I've pushed the patch to master.

(In reply to comment #11)
> Created an attachment (id=202271) [details]
> Modified version of text-intro-03-b
> 
> (In reply to comment #9)
> > Created an attachment (id=202194) [details] [details]
> > test rendering
> > 
> > The final dots are placed differently than in attachment 201898 [details] [details], and I wasn't
> > sure which rendering is right.
> 
> Some characters (including dots) have two glyphs.
> One is for horizontal writing. And the other is for vertical.
> http://en.wikipedia.org/wiki/File:Tateyoko.png
> 
> However some fonts don't have glyphs for vertical writing.
> Thus dots placement can vary.

I guess the same thing results in the middle text column exceeding the image height? The text runs off the bottom at the ま and the せん。are invisible, completely off the bottom.

> (In reply to comment #10)
> > Also the rendering of
> > http://www.w3.org/Graphics/SVG/Test/20110816/harness/htmlObjectApproved/text-intro-03-b.html
> > isn't right.
> 
> The test case doesn't include language information (i.e. xml:lang attributes).
> So font substitution and rendering can fail if pango fails to detect languages. 
> I attached modified version of test case which has xml:lang attributes for each
> text elements.
> This would not produce mis-rendering.


I guess I just don't have the fonts, since the chinese text at the left still is all printed all glyphs on top of each other.
 
> The right two sentences "Se: 私は"/"Japanese: 私は..." use
> glyph-orientation-vertical property.
> Since the patch doesn't support that property, these are rendered likely
> center two sentences "Se: 私は"/"Japanese: 私は...".

Is that easy to implement? If so I'll leave this bug open for that, otherwise let's file that as a new one.
Comment 15 Kurosawa Takeshi 2011-11-29 07:21:02 UTC
Created attachment 202348 [details] [review]
Partial support of glyph-orientation-{vertical, horizontal}

This patch adds partial support of glyph-orientation-{vertical, horizontal}.

Correctly rendered:
glyph-orientation-vertical: auto, 0, and 90;
glyph-orientation-horizontal: 0, and 270;


Not correctly rendered:
glyph-orientation-vertical: 180 and 270;
glyph-orientation-horizontal: 90 and 180;
Comment 16 Kurosawa Takeshi 2011-11-29 07:43:00 UTC
Created attachment 202349 [details]
Testcase for glyph-orientation-{vertical, horizontal}
Comment 17 Kurosawa Takeshi 2011-11-29 07:46:51 UTC
Created attachment 202350 [details]
Rendering result with patch

Rendering result of attachment 202349 [details] with patch
Comment 18 Kurosawa Takeshi 2011-11-29 07:52:40 UTC
Created attachment 202351 [details]
Rendering result from Chromium browser (correct)

Chromium browser's rendering of attachment 202349 [details].
This is expected result.

Comparing this, librsvg with patch rotates texts instead of glyphs
if glyph-orientation-vertical is 180 or 270 and glyph-orientation-horizontal is 90 and 180.
Comment 19 Christian Persch 2011-12-03 18:09:22 UTC
Thanks for the patch!

Did you attach the testcase incorrectly? Attachment 202349 [details] is a duplicate of the 'rendering result' attachment 202350 [details].

+            double degree = rsvg_css_parse_angle (value);
+            degree = fmod(degree, 360.);
+            degree = degree < 0 ? 360. + degree : degree;

I think these two extra lines should go into rsvg_css_parse_angle().

+typedef enum {
+  GLYPH_ORIENTATION_VERTICAL_AUTO = PANGO_GRAVITY_AUTO,
+  GLYPH_ORIENTATION_VERTICAL_0 = PANGO_GRAVITY_EAST,
+  GLYPH_ORIENTATION_VERTICAL_90 = PANGO_GRAVITY_SOUTH,
+  /* XXX Below two values dopn't match SVG 1.1 Spec. */
+  GLYPH_ORIENTATION_VERTICAL_180 = PANGO_GRAVITY_WEST,
+  GLYPH_ORIENTATION_VERTICAL_270 = PANGO_GRAVITY_NORTH
+} GlyphOrientationVertical;
+
+typedef enum {
+  GLYPH_ORIENTATION_HORIZONTAL_0 = PANGO_GRAVITY_SOUTH,
+  /* XXX Below two values don't match SVG 1.1 Spec. */
+  GLYPH_ORIENTATION_HORIZONTAL_90 = PANGO_GRAVITY_WEST,
+  GLYPH_ORIENTATION_HORIZONTAL_180 = PANGO_GRAVITY_NORTH,
+  GLYPH_ORIENTATION_HORIZONTAL_270 = PANGO_GRAVITY_EAST
+} GlyphOrientationHorizontal;

Instead of assigning the new enums to the PangoGravity values, I think the enum should just have auto-values and the PangoGravity and PangoGravityHint be retrieved from a function get_[horiz|vert]_gravity_and_hint (GlyphOrientation[Horizontal|Vertical] orient, PangoGravity *grav, PangoGravityHint *hint). That makes it easier to understand, IMHO.

-    iter = pango_layout_get_iter (layout);
+    /* Align characters to the natural baseline */
+    tmp_layout = pango_layout_copy(layout);
+    pango_context_set_base_gravity(context, state->text_gravity);
+    pango_context_set_gravity_hint(context, PANGO_GRAVITY_HINT_NATURAL);
+    pango_layout_context_changed (tmp_layout);
+    iter = pango_layout_get_iter (tmp_layout);

Could you explaing why you need to make a copy here and change the gravity?

Also, why the pango_layout_context_changed() ­— from its docs I don't see why it would be necessary here?
Comment 20 Christian Persch 2012-01-13 20:35:42 UTC
Ping, any updates?
Comment 21 Christian Persch 2012-09-05 10:43:38 UTC
Ping again?
Comment 22 Kurosawa Takeshi 2012-09-05 13:23:44 UTC
Sorry for long delay.
I'll be look at in next Saturday and Sunday.
Comment 23 Kurosawa Takeshi 2012-09-09 15:05:04 UTC
Created attachment 223854 [details] [review]
Updated patch for glyph-orientation-{vertical, horizontal}

Addressed review comments.
Changed to use default value's gravity/gravity_hint for unsupported values.
Comment 24 André Klapper 2016-03-08 20:13:03 UTC
(In reply to Kurosawa Takeshi from comment #23)
> Created attachment 223854 [details] [review] [review]
> Updated patch for glyph-orientation-{vertical, horizontal}
> 
> Addressed review comments.
> Changed to use default value's gravity/gravity_hint for unsupported values.

Wondering if this could get a review?
Comment 25 GNOME Infrastructure Team 2017-12-13 17:50:26 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/librsvg/issues/55.