[librsvg: 1/12] state: move direction and writing mode to rust

[Federico Mena Quintero <federico src gnome org>]

commit dbb23aae044f72d541db73fb9fb1aef2084df46b
Author: Paolo Borelli <pborelli gnome org>
Date:   Sat Apr 21 10:24:21 2018 +0200

    state: move direction and writing mode to rust
    
    Note that the logic used to set the direction on the pango context
    is changed: in the C code, whichever of the two properties was set
    last defined text_dir. Instead now we keep both in the state, then
    when creating the pango layout we set the default dir in this way:
     - if bidi is set to override, from the direction prop (including
       its default)
     - otherwise if direction is specified, it wins
     - otherwise we default to the direction associated with the
       writing mode
    
    I am not sure this is correct, but at least the logic is now in
    one place, and it seems less aleatory than what was there before.
    The test suite passes.

 librsvg/rsvg-styles.c       | 57 ---------------------------
 librsvg/rsvg-styles.h       | 13 -------
 rsvg_internals/src/draw.rs  | 11 +++++-
 rsvg_internals/src/state.rs | 58 +++++++++++++++++++++------
 rsvg_internals/src/text.rs  | 95 +++++++++++++++++++++++++++++++--------------
 5 files changed, 120 insertions(+), 114 deletions(-)
---
diff --git a/librsvg/rsvg-styles.c b/librsvg/rsvg-styles.c
index de32a311..0477e8c7 100644
--- a/librsvg/rsvg-styles.c
+++ b/librsvg/rsvg-styles.c
@@ -113,8 +113,6 @@ rsvg_state_init (RsvgState * state)
     state->flood_color = 0;
     state->flood_opacity = 255;
 
-    state->text_dir = PANGO_DIRECTION_LTR;
-    state->text_gravity = PANGO_GRAVITY_SOUTH;
     state->cond_true = TRUE;
 
     state->has_current_color = FALSE;
@@ -127,8 +125,6 @@ rsvg_state_init (RsvgState * state)
     state->has_cond = FALSE;
     state->has_stop_color = FALSE;
     state->has_stop_opacity = FALSE;
-    state->has_text_dir = FALSE;
-    state->has_text_gravity = FALSE;
 
     state->state_rust = rsvg_state_rust_new();
 }
@@ -252,10 +248,6 @@ rsvg_state_inherit_run (RsvgState * dst, const RsvgState * src,
     }
     if (function (dst->has_cond, src->has_cond))
         dst->cond_true = src->cond_true;
-    if (function (dst->has_text_dir, src->has_text_dir))
-        dst->text_dir = src->text_dir;
-    if (function (dst->has_text_gravity, src->has_text_gravity))
-        dst->text_gravity = src->text_gravity;
 
     rsvg_state_rust_inherit_run (dst->state_rust, src->state_rust, function, inherituninheritables);
 
@@ -516,43 +508,6 @@ rsvg_parse_style_pair (RsvgState *state,
     }
     break;
 
-    case RSVG_ATTRIBUTE_DIRECTION:
-    {
-        state->has_text_dir = TRUE;
-        if (g_str_equal (value, "inherit")) {
-            state->text_dir = PANGO_DIRECTION_LTR;
-            state->has_text_dir = FALSE;
-        } else if (g_str_equal (value, "rtl"))
-            state->text_dir = PANGO_DIRECTION_RTL;
-        else                    /* ltr */
-            state->text_dir = PANGO_DIRECTION_LTR;
-    }
-    break;
-
-    case RSVG_ATTRIBUTE_WRITING_MODE:
-    {
-        /* TODO: these aren't quite right... */
-
-        state->has_text_dir = TRUE;
-        state->has_text_gravity = TRUE;
-        if (g_str_equal (value, "inherit")) {
-            state->text_dir = PANGO_DIRECTION_LTR;
-            state->has_text_dir = FALSE;
-            state->text_gravity = PANGO_GRAVITY_SOUTH;
-            state->has_text_gravity = FALSE;
-        } else if (g_str_equal (value, "lr-tb") || g_str_equal (value, "lr")) {
-            state->text_dir = PANGO_DIRECTION_LTR;
-            state->text_gravity = PANGO_GRAVITY_SOUTH;
-        } else if (g_str_equal (value, "rl-tb") || g_str_equal (value, "rl")) {
-            state->text_dir = PANGO_DIRECTION_RTL;
-            state->text_gravity = PANGO_GRAVITY_SOUTH;
-        } else if (g_str_equal (value, "tb-rl") || g_str_equal (value, "tb")) {
-            state->text_dir = PANGO_DIRECTION_LTR;
-            state->text_gravity = PANGO_GRAVITY_EAST;
-        }
-    }
-    break;
-
     case RSVG_ATTRIBUTE_STOP_COLOR:
     {
         state->has_stop_color = TRUE;
@@ -1249,18 +1204,6 @@ rsvg_state_get_current_color (RsvgState *state)
     return state->current_color;
 }
 
-PangoDirection
-rsvg_state_get_text_dir (RsvgState *state)
-{
-    return state->text_dir;
-}
-
-PangoGravity
-rsvg_state_get_text_gravity (RsvgState *state)
-{
-    return state->text_gravity;
-}
-
 RsvgPaintServer *
 rsvg_state_get_fill (RsvgState *state)
 {
diff --git a/librsvg/rsvg-styles.h b/librsvg/rsvg-styles.h
index 4e4bc23c..4f48d341 100644
--- a/librsvg/rsvg-styles.h
+++ b/librsvg/rsvg-styles.h
@@ -60,13 +60,6 @@ struct _RsvgState {
     guint8 stroke_opacity;      /* 0..255 */
     gboolean has_stroke_opacity;
 
-    PangoDirection text_dir;
-    gboolean has_text_dir;
-    PangoGravity text_gravity;
-    gboolean has_text_gravity;
-
-    guint text_offset;
-
     RsvgCssColorSpec stop_color;
     gboolean has_stop_color;
 
@@ -170,12 +163,6 @@ RsvgOpacitySpec *rsvg_state_get_stop_opacity (RsvgState *state);
 G_GNUC_INTERNAL
 guint32 rsvg_state_get_current_color (RsvgState *state);
 
-G_GNUC_INTERNAL
-PangoDirection rsvg_state_get_text_dir (RsvgState *state);
-
-G_GNUC_INTERNAL
-PangoGravity rsvg_state_get_text_gravity (RsvgState *state);
-
 G_GNUC_INTERNAL
 RsvgPaintServer *rsvg_state_get_fill (RsvgState *state);
 
diff --git a/rsvg_internals/src/draw.rs b/rsvg_internals/src/draw.rs
index 4b71da60..371afd80 100644
--- a/rsvg_internals/src/draw.rs
+++ b/rsvg_internals/src/draw.rs
@@ -25,7 +25,6 @@ use state::{
     StrokeWidth,
     TextRendering,
 };
-use text;
 
 pub fn draw_path_builder(draw_ctx: *mut RsvgDrawingCtx, builder: &PathBuilder, clipping: bool) {
     if !clipping {
@@ -399,6 +398,14 @@ pub fn draw_pango_layout(
     cr.restore();
 }
 
+// FIXME: should the pango crate provide this like PANGO_GRAVITY_IS_VERTICAL() ?
+fn gravity_is_vertical(gravity: pango::Gravity) -> bool {
+    match gravity {
+        pango::Gravity::East | pango::Gravity::West => true,
+        _ => false,
+    }
+}
+
 fn compute_text_bbox(
     ink: &pango::Rectangle,
     x: f64,
@@ -415,7 +422,7 @@ fn compute_text_bbox(
     let ink_width = f64::from(ink.width);
     let ink_height = f64::from(ink.height);
 
-    if text::gravity_is_vertical(gravity) {
+    if gravity_is_vertical(gravity) {
         bbox.set_rect(&cairo::Rectangle {
             x: x + (ink_x - ink_height) / pango_scale,
             y: y + ink_y / pango_scale,
diff --git a/rsvg_internals/src/state.rs b/rsvg_internals/src/state.rs
index 8098b1ed..e6c4ca0c 100644
--- a/rsvg_internals/src/state.rs
+++ b/rsvg_internals/src/state.rs
@@ -2,8 +2,6 @@ use cairo::{self, MatrixTrait};
 use glib::translate::*;
 use glib_sys;
 use libc;
-use pango;
-use pango_sys;
 use std::cell::RefCell;
 use std::collections::HashSet;
 use std::ptr;
@@ -43,6 +41,7 @@ pub struct State {
     pub clip_path: Option<ClipPath>,
     pub clip_rule: Option<ClipRule>,
     pub comp_op: Option<CompOp>,
+    pub direction: Option<Direction>,
     pub display: Option<Display>,
     pub enable_background: Option<EnableBackground>,
     pub fill_rule: Option<FillRule>,
@@ -71,6 +70,7 @@ pub struct State {
     pub text_rendering: Option<TextRendering>,
     pub unicode_bidi: Option<UnicodeBidi>,
     pub visibility: Option<Visibility>,
+    pub writing_mode: Option<WritingMode>,
     pub xml_lang: Option<XmlLang>,
     pub xml_space: Option<XmlSpace>,
 
@@ -87,6 +87,7 @@ impl State {
             clip_path: Default::default(),
             clip_rule: Default::default(),
             comp_op: Default::default(),
+            direction: Default::default(),
             display: Default::default(),
             enable_background: Default::default(),
             fill_rule: Default::default(),
@@ -115,6 +116,7 @@ impl State {
             text_rendering: Default::default(),
             unicode_bidi: Default::default(),
             visibility: Default::default(),
+            writing_mode: Default::default(),
             xml_lang: Default::default(),
             xml_space: Default::default(),
 
@@ -146,6 +148,10 @@ impl State {
                 self.comp_op = parse_property(value, ())?;
             }
 
+            Attribute::Direction => {
+                self.direction = parse_property(value, ())?;
+            }
+
             Attribute::Display => {
                 self.display = parse_property(value, ())?;
             }
@@ -272,6 +278,10 @@ impl State {
                 self.visibility = parse_property(value, ())?;
             }
 
+            Attribute::WritingMode => {
+                self.writing_mode = parse_property(value, ())?;
+            }
+
             Attribute::XmlLang => {
                 // xml:lang is not a property; it is a non-presentation attribute and as such
                 // cannot have the "inherit" value.  So, we don't call parse_property() for it,
@@ -326,8 +336,6 @@ extern "C" {
     fn rsvg_state_get_current_color(state: *const RsvgState) -> u32;
     fn rsvg_state_get_stroke(state: *const RsvgState) -> *const PaintServer;
     fn rsvg_state_get_stroke_opacity(state: *const RsvgState) -> u8;
-    fn rsvg_state_get_text_dir(state: *const RsvgState) -> pango_sys::PangoDirection;
-    fn rsvg_state_get_text_gravity(state: *const RsvgState) -> pango_sys::PangoGravity;
     fn rsvg_state_get_fill(state: *const RsvgState) -> *const PaintServer;
     fn rsvg_state_get_fill_opacity(state: *const RsvgState) -> u8;
 
@@ -403,6 +411,15 @@ pub fn is_visible(state: *const RsvgState) -> bool {
     }
 }
 
+pub fn text_gravity_is_vertical(state: *const RsvgState) -> bool {
+    let rstate = get_state_rust(state);
+
+    match rstate.writing_mode {
+        Some(WritingMode::Tb) | Some(WritingMode::TbRl) => true,
+        _ => false,
+    }
+}
+
 pub fn get_cond_true(state: *const RsvgState) -> bool {
     unsafe { from_glib(rsvg_state_get_cond_true(state)) }
 }
@@ -459,14 +476,6 @@ pub fn get_stroke_opacity(state: *const RsvgState) -> u8 {
     unsafe { rsvg_state_get_stroke_opacity(state) }
 }
 
-pub fn get_text_dir(state: *const RsvgState) -> pango::Direction {
-    unsafe { from_glib(rsvg_state_get_text_dir(state)) }
-}
-
-pub fn get_text_gravity(state: *const RsvgState) -> pango::Gravity {
-    unsafe { from_glib(rsvg_state_get_text_gravity(state)) }
-}
-
 pub fn get_fill<'a>(state: *const RsvgState) -> Option<&'a PaintServer> {
     unsafe {
         let ps = rsvg_state_get_fill(state);
@@ -581,6 +590,16 @@ make_property!(
     "exclusion" => Exclusion,
 );
 
+make_property!(
+    Direction,
+    default: Ltr,
+    inherits_automatically: true,
+
+    identifiers:
+    "ltr" => Ltr,
+    "rtl" => Rtl,
+);
+
 make_property!(
     Display,
     default: Inline,
@@ -895,6 +914,20 @@ make_property!(
     "collapse" => Collapse,
 );
 
+make_property!(
+    WritingMode,
+    default: LrTb,
+    inherits_automatically: true,
+
+    identifiers:
+    "lr" => Lr,
+    "lr-tb" => LrTb,
+    "rl" => Rl,
+    "rl-tb" => RlTb,
+    "tb" => Tb,
+    "tb-rl" => TbRl,
+);
+
 make_property!(
     XmlLang,
     default: "C".to_string(),
@@ -1030,6 +1063,7 @@ pub extern "C" fn rsvg_state_rust_inherit_run(
     // please keep these sorted
     inherit(inherit_fn, &mut dst.baseline_shift, &src.baseline_shift);
     inherit(inherit_fn, &mut dst.clip_rule, &src.clip_rule);
+    inherit(inherit_fn, &mut dst.direction, &src.direction);
     inherit(inherit_fn, &mut dst.display, &src.display);
     inherit(inherit_fn, &mut dst.fill_rule, &src.fill_rule);
     inherit(inherit_fn, &mut dst.font_family, &src.font_family);
diff --git a/rsvg_internals/src/text.rs b/rsvg_internals/src/text.rs
index 4d5423b6..e0c30d0e 100644
--- a/rsvg_internals/src/text.rs
+++ b/rsvg_internals/src/text.rs
@@ -23,6 +23,7 @@ use property_bag::PropertyBag;
 use space::xml_space_normalize;
 use state::{
     self,
+    Direction,
     FontFamily,
     FontStretch,
     FontStyle,
@@ -32,6 +33,7 @@ use state::{
     RsvgState,
     TextAnchor,
     UnicodeBidi,
+    WritingMode,
     XmlLang,
 };
 
@@ -98,8 +100,7 @@ impl NodeChars {
         let baseline = f64::from(layout.get_baseline()) / f64::from(pango::SCALE);
         let offset = baseline + drawing_ctx::get_accumulated_baseline_shift(draw_ctx);
 
-        let gravity = state::get_text_gravity(state);
-        if gravity_is_vertical(gravity) {
+        if state::text_gravity_is_vertical(state) {
             draw_pango_layout(draw_ctx, &layout, *x + offset, *y, clipping);
             *y += f64::from(width) / f64::from(pango::SCALE);
         } else {
@@ -167,11 +168,10 @@ impl NodeTrait for NodeText {
 
         let state = drawing_ctx::get_current_state(draw_ctx);
         let anchor = state::get_state_rust(state).text_anchor.unwrap_or_default();
-        let gravity = state::get_text_gravity(state);
 
         let offset = anchor_offset(node, draw_ctx, anchor, false);
 
-        if gravity_is_vertical(gravity) {
+        if state::text_gravity_is_vertical(state) {
             y -= offset;
             dy = match anchor {
                 TextAnchor::Start => dy,
@@ -291,8 +291,7 @@ impl NodeTSpan {
         }
 
         let state = drawing_ctx::get_current_state(draw_ctx);
-        let gravity = state::get_text_gravity(state);
-        if gravity_is_vertical(gravity) {
+        if state::text_gravity_is_vertical(state) {
             *length += self.dy.get().normalize(draw_ctx);
         } else {
             *length += self.dx.get().normalize(draw_ctx);
@@ -317,14 +316,14 @@ impl NodeTSpan {
         let mut dy = self.dy.get().normalize(draw_ctx);
 
         let state = drawing_ctx::get_current_state(draw_ctx);
+        let vertical = state::text_gravity_is_vertical(state);
         let anchor = state::get_state_rust(state).text_anchor.unwrap_or_default();
-        let gravity = state::get_text_gravity(state);
 
         let offset = anchor_offset(node, draw_ctx, anchor, usetextonly);
 
         if let Some(self_x) = self.x.get() {
             *x = self_x.normalize(draw_ctx);
-            if !gravity_is_vertical(gravity) {
+            if !vertical {
                 *x -= offset;
                 dx = match anchor {
                     TextAnchor::Start => dx,
@@ -337,7 +336,7 @@ impl NodeTSpan {
 
         if let Some(self_y) = self.y.get() {
             *y = self_y.normalize(draw_ctx);
-            if gravity_is_vertical(gravity) {
+            if vertical {
                 *y -= offset;
                 dy = match anchor {
                     TextAnchor::Start => dy,
@@ -381,15 +380,6 @@ impl NodeTrait for NodeTSpan {
     }
 }
 
-// FIXME: should the pango crate provide this like PANGO_GRAVITY_IS_VERTICAL() /
-// PANGO_GRAVITY_IS_IMPROPER()?
-pub fn gravity_is_vertical(gravity: pango::Gravity) -> bool {
-    match gravity {
-        pango::Gravity::East | pango::Gravity::West => true,
-        _ => false,
-    }
-}
-
 fn to_pango_units(v: f64) -> i32 {
     (v * f64::from(pango::SCALE)) as i32
 }
@@ -451,6 +441,46 @@ impl From<FontWeight> for pango::Weight {
     }
 }
 
+impl From<Direction> for pango::Direction {
+    fn from(d: Direction) -> pango::Direction {
+        match d {
+            Direction::Ltr => pango::Direction::Ltr,
+            Direction::Rtl => pango::Direction::Rtl,
+        }
+    }
+}
+
+impl From<Direction> for pango::Alignment {
+    fn from(d: Direction) -> pango::Alignment {
+        match d {
+            Direction::Ltr => pango::Alignment::Left,
+            Direction::Rtl => pango::Alignment::Right,
+        }
+    }
+}
+
+impl From<WritingMode> for pango::Direction {
+    fn from(m: WritingMode) -> pango::Direction {
+        match m {
+            WritingMode::LrTb | WritingMode::Lr | WritingMode::Tb | WritingMode::TbRl => {
+                pango::Direction::Ltr
+            }
+            WritingMode::RlTb | WritingMode::Rl => pango::Direction::Rtl,
+        }
+    }
+}
+
+impl From<WritingMode> for pango::Gravity {
+    fn from(m: WritingMode) -> pango::Gravity {
+        match m {
+            WritingMode::Tb | WritingMode::TbRl => pango::Gravity::East,
+            WritingMode::LrTb | WritingMode::Lr | WritingMode::RlTb | WritingMode::Rl => {
+                pango::Gravity::South
+            }
+        }
+    }
+}
+
 fn create_pango_layout(draw_ctx: *const RsvgDrawingCtx, text: &str) -> pango::Layout {
     let state = drawing_ctx::get_current_state(draw_ctx);
     let rstate = state::get_state_rust(state);
@@ -461,17 +491,25 @@ fn create_pango_layout(draw_ctx: *const RsvgDrawingCtx, text: &str) -> pango::La
         pango_context.set_language(&pango_lang);
     }
 
-    match rstate.unicode_bidi {
-        Some(UnicodeBidi::Override) | Some(UnicodeBidi::Embed) => {
-            pango_context.set_base_dir(state::get_text_dir(state));
+    pango_context.set_base_gravity(pango::Gravity::from(
+        rstate.writing_mode.unwrap_or_default(),
+    ));
+
+    match (rstate.unicode_bidi, rstate.direction) {
+        (Some(UnicodeBidi::Override), _) | (Some(UnicodeBidi::Embed), _) => {
+            pango_context
+                .set_base_dir(pango::Direction::from(rstate.direction.unwrap_or_default()));
         }
 
-        _ => (),
-    }
+        (_, Some(direction)) => {
+            pango_context.set_base_dir(pango::Direction::from(direction));
+        }
 
-    let gravity = state::get_text_gravity(state);
-    if gravity_is_vertical(gravity) {
-        pango_context.set_base_gravity(gravity);
+        (_, _) => {
+            pango_context.set_base_dir(pango::Direction::from(
+                rstate.writing_mode.unwrap_or_default(),
+            ));
+        }
     }
 
     let mut font_desc = pango_context.get_font_description().unwrap();
@@ -520,10 +558,7 @@ fn create_pango_layout(draw_ctx: *const RsvgDrawingCtx, text: &str) -> pango::La
 
     layout.set_attributes(&attr_list);
 
-    layout.set_alignment(match state::get_text_dir(state) {
-        pango::Direction::Ltr => pango::Alignment::Left,
-        _ => pango::Alignment::Right,
-    });
+    layout.set_alignment(pango::Alignment::from(rstate.direction.unwrap_or_default()));
 
     let t = xml_space_normalize(rstate.xml_space.unwrap_or_default(), text);
     layout.set_text(&t);