CSS2 and CSS3 text decoration rendering, most code from David Mathog.

(bzr r13372)

4 files changed, 427 insertions, 199 deletions

diff --git a/src/display/drawing-text.cpp b/src/display/drawing-text.cpp
index 4178cb1d8..05a2c3c2a 100644
--- a/src/display/drawing-text.cpp
+++ b/src/display/drawing-text.cpp
@@ -67,16 +67,14 @@ unsigned DrawingGlyphs::_updateItem(Geom::IntRect const &/*area*/, UpdateContext
     _pick_bbox = Geom::IntRect();
     _bbox = Geom::IntRect();
 
-/* orignally it did the one line below,
-    but it did not handle ws characters at all, and it had problems with scaling for overline/underline.
-    Replaced with the section below, which seems to be much more stable.
-
-    Geom::OptRect b  = bounds_exact_transformed(*_font->PathVector(_glyph), ctx.ctm);
-*/
-    /* Make a bounding box that is a little taller and lower (currently 10% extra) than the font's drawing box.  Extra space is
-    to hold overline or underline, if present.  All characters in a font use the same ascent and descent,
-    but different widths. This lets leading and trailing spaces have text decorations. If it is not done 
-    the bounding box is limited to the box surrounding the drawn parts of visible glyphs only, and draws outside are ignored.
+    /*
+      Make a bounding box for drawing that is a little taller and lower (currently 10% extra) than
+      the font's drawing box.  Extra space is to hold overline or underline, if present.  All
+      characters in a font use the same ascent and descent, but different widths. This lets leading
+      and trailing spaces have text decorations. If it is not done the bounding box is limited to
+      the box surrounding the drawn parts of visible glyphs only, and draws outside are ignored.
+      The box is also a hair wider than the text, since the glyphs do not always start or end at
+      the left and right edges of the box defined in the font.
     */
 
     float scale_bigbox = 1.0;
@@ -84,9 +82,45 @@ unsigned DrawingGlyphs::_updateItem(Geom::IntRect const &/*area*/, UpdateContext
         scale_bigbox /= _transform->descrim();
     }
 
-    Geom::Rect bigbox(Geom::Point(0.0, _asc*scale_bigbox*1.1),Geom::Point(_width*scale_bigbox, -_dsc*scale_bigbox*1.1));
+    Geom::Rect bigbox(Geom::Point(-_width*scale_bigbox*0.1, _asc*scale_bigbox*1.1),Geom::Point(_width*scale_bigbox, -_dsc*scale_bigbox*1.1));
     Geom::Rect b = bigbox * ctx.ctm;
 
+    /*
+      The pick box matches the characters as best as it can, leaving no extra space above or below
+      for decorations.  The pathvector may include spaces, and spaces have no drawable glyph.
+      Catch those and do not pass them to bounds_exact_transformed(), which crashes Inkscape if it
+      sees a nondrawable glyph. Instead mock up a pickbox for them using font characteristics.
+      There may also be some other similar white space characters in some other unforeseen context
+      which should be handled by this code as well..
+    */
+
+    Geom::OptRect pb;
+    if(_drawable){
+        pb  = bounds_exact_transformed(*_font->PathVector(_glyph), ctx.ctm);
+    }
+    if(!pb){ // Fallback
+        Geom::Rect pbigbox(Geom::Point(0.0, _asc*scale_bigbox*0.66),Geom::Point(_width*scale_bigbox, 0.0));
+        pb = pbigbox * ctx.ctm;
+    }
+
+#if 0
+    /* FIXME  if this is commented out then not even an approximation of pick on decorations */
+    /* adjust the pick box up or down to include the decorations.  
+       This is only approximate since at this point we don't know how wide that line is, if it has
+       an unusual offset, and so forth.  The selection point is set at what is roughly the center of
+       the decoration (vertically) for the wide ones, like wavy and double line.
+       The text decorations are not actually selectable.
+    */
+    if (_decorations.overline || _decorations.underline) {
+        double top = _asc*scale_bigbox*0.66;
+        double bot = 0;
+        if (_decorations.overline) {  top =   _asc * scale_bigbox * 1.025; }
+        if (_decorations.underline) { bot =  -_dsc * scale_bigbox * 0.2;   }
+        Geom::Rect padjbox(Geom::Point(0.0, top),Geom::Point(_width*scale_bigbox, bot));
+        pb.unionWith(padjbox * ctx.ctm);
+    }
+#endif   
+
     if (ggroup->_nrstyle.stroke.type != NRStyle::PAINT_NONE) {
         // this expands the selection box for cases where the stroke is "thick"
         float scale = ctx.ctm.descrim();
@@ -96,10 +130,11 @@ unsigned DrawingGlyphs::_updateItem(Geom::IntRect const &/*area*/, UpdateContext
         float width = MAX(0.125, ggroup->_nrstyle.stroke_width * scale);
         if ( fabs(ggroup->_nrstyle.stroke_width * scale) > 0.01 ) { // FIXME: this is always true
             b.expandBy(0.5 * width);
+            pb->expandBy(0.5 * width);
         }
 
        // save bbox without miters for picking
-        _pick_bbox = b.roundOutwards();
+        _pick_bbox = pb->roundOutwards();
 
         float miterMax = width * ggroup->_nrstyle.miter_limit;
         if ( miterMax > 0.01 ) {
@@ -110,14 +145,8 @@ unsigned DrawingGlyphs::_updateItem(Geom::IntRect const &/*area*/, UpdateContext
         _bbox = b.roundOutwards();
     } else {
         _bbox = b.roundOutwards();
-        _pick_bbox = *_bbox;
+        _pick_bbox = pb->roundOutwards();
     }
-/*
-std::cout << "DEBUG _bbox"
-<< " { " << _bbox->min()[Geom::X] << " , " << _bbox->min()[Geom::Y]
-<< " } , { " << _bbox->max()[Geom::X] << " , " << _bbox->max()[Geom::Y]
-<< " }" << std::endl;
-*/
     return STATE_ALL;
 }
 
@@ -136,7 +165,8 @@ DrawingGlyphs::_pickItem(Geom::Point const &p, double delta, unsigned /*flags*/)
 
     // With text we take a simple approach: pick if the point is in a character bbox
     Geom::Rect expanded(_pick_bbox);
-    expanded.expandBy(delta);
+    // FIXME, why expand by delta?  When is the next line needed?
+    // expanded.expandBy(delta);
     if (expanded.contains(p)) return this;
     return NULL;
 }
@@ -195,7 +225,7 @@ DrawingText::_updateItem(Geom::IntRect const &area, UpdateContext const &ctx, un
     return DrawingGroup::_updateItem(area, ctx, flags, reset);
 }
 
-void DrawingText::decorateStyle(DrawingContext &dc, double vextent, double xphase, Geom::Point const &p1, Geom::Point const &p2)
+void DrawingText::decorateStyle(DrawingContext &dc, double vextent, double xphase, Geom::Point const &p1, Geom::Point const &p2, double thickness)
 {
     double wave[16]={
         0.000000,  0.382499,  0.706825,  0.923651,   1.000000,  0.923651,  0.706825,  0.382499, 
@@ -213,7 +243,6 @@ void DrawingText::decorateStyle(DrawingContext &dc, double vextent, double xphas
         4,     3,   2,   1,
         -4,   -3,  -2,  -1
     };
-    Geom::Point p3,p4,ps,pf;
     double   step = vextent/32.0;
     unsigned i  = 15 & (unsigned) round(xphase/step);  // xphase is >= 0.0
 
@@ -221,26 +250,19 @@ void DrawingText::decorateStyle(DrawingContext &dc, double vextent, double xphas
        This allows decoration continuity within the line, and does not step outside the clip box off the end
        For the first/last section on the line though, stay well clear of the edge, or when the 
        text is dragged it may "spray" pixels.
-    if(_nrstyle.tspan_line_end){    pf = p2 - Geom::Point(2*step, 0.0); }
-    else {                          pf = p2;                            }
-    if(_nrstyle.tspan_line_start){  ps = p1 + Geom::Point(2*step, 0.0);
-                                    i  = 15 & (i + 2); 
-    }
-    else {                          ps = p1;                            }
     */
     /* snap to nearest step in X */
-ps = Geom::Point(step * round(p1[Geom::X]/step),p1[Geom::Y]);
-pf = Geom::Point(step * round(p2[Geom::X]/step),p2[Geom::Y]);
+    Geom::Point ps = Geom::Point(step * round(p1[Geom::X]/step),p1[Geom::Y]);
+    Geom::Point pf = Geom::Point(step * round(p2[Geom::X]/step),p2[Geom::Y]);
+    Geom::Point poff = Geom::Point(0,thickness/2.0);
 
     if(_nrstyle.text_decoration_style & TEXT_DECORATION_STYLE_ISDOUBLE){
         ps -= Geom::Point(0, vextent/12.0);
         pf -= Geom::Point(0, vextent/12.0);
-        dc.moveTo(ps);
-        dc.lineTo(pf);
+        dc.rectangle( Geom::Rect(ps + poff, pf - poff));
         ps += Geom::Point(0, vextent/6.0);
         pf += Geom::Point(0, vextent/6.0);
-        dc.moveTo(ps);
-        dc.lineTo(pf);
+        dc.rectangle( Geom::Rect(ps + poff, pf - poff));
     }
     /* The next three have a problem in that they are phase dependent.  The bits of a line are not
     necessarily passing through this routine in order, so we have to use the xphase information
@@ -248,43 +270,52 @@ pf = Geom::Point(step * round(p2[Geom::X]/step),p2[Geom::Y]);
     Huge possitive offset should keep the phase calculation from ever being negative.
     */
     else if(_nrstyle.text_decoration_style & TEXT_DECORATION_STYLE_DOTTED){
+        // FIXME: Per spec, this should produce round dots.
+        Geom::Point pv = ps;
         while(1){
+            Geom::Point pvlast = pv;
             if(dots[i]>0){
-                if(ps[Geom::X]> pf[Geom::X])break;
-                dc.moveTo(ps);
-                ps += Geom::Point(step * (double)dots[i], 0.0);
-                if(ps[Geom::X]>= pf[Geom::X]){
-                    dc.lineTo(pf);
+                if(pv[Geom::X] > pf[Geom::X]) break;
+
+                pv += Geom::Point(step * (double)dots[i], 0.0);
+
+                if(pv[Geom::X]>= pf[Geom::X]){
+                    // Last dot
+                    dc.rectangle( Geom::Rect(pvlast + poff, pf - poff));
                     break;
+                } else {
+                    dc.rectangle( Geom::Rect(pvlast + poff, pv - poff));
                 }
-                else {
-                    dc.lineTo(ps);
-                }
-                ps += Geom::Point(step * 4.0, 0.0);
-            }
-            else {
-                ps += Geom::Point(step * -(double)dots[i], 0.0);
+
+                pv += Geom::Point(step * 4.0, 0.0);
+
+            } else {
+                pv += Geom::Point(step * -(double)dots[i], 0.0);
             }
             i = 0;  // once in phase, it stays in phase
         }
     } 
     else if(_nrstyle.text_decoration_style & TEXT_DECORATION_STYLE_DASHED){
+        Geom::Point pv = ps;
         while(1){
+            Geom::Point pvlast = pv;
             if(dashes[i]>0){
-                if(ps[Geom::X]> pf[Geom::X])break;
-                dc.moveTo(ps);
-                ps += Geom::Point(step * (double)dashes[i], 0.0);
-                if(ps[Geom::X]>= pf[Geom::X]){
-                    dc.lineTo(pf);
+                if(pv[Geom::X]> pf[Geom::X]) break;
+
+                pv += Geom::Point(step * (double)dashes[i], 0.0);
+
+                if(pv[Geom::X]>= pf[Geom::X]){
+                    // Last dash
+                    dc.rectangle( Geom::Rect(pvlast + poff, pf - poff));
                     break;
+                } else {
+                    dc.rectangle( Geom::Rect(pvlast + poff, pv - poff));
                 }
-                else {
-                    dc.lineTo(ps);
-                }
-                ps += Geom::Point(step * 8.0, 0.0);
-            }
-            else {
-                ps += Geom::Point(step * -(double)dashes[i], 0.0);
+
+                pv += Geom::Point(step * 8.0, 0.0);
+
+            } else {
+                pv += Geom::Point(step * -(double)dashes[i], 0.0);
             }
             i = 0;  // once in phase, it stays in phase
         }
@@ -292,7 +323,7 @@ pf = Geom::Point(step * round(p2[Geom::X]/step),p2[Geom::Y]);
     else if(_nrstyle.text_decoration_style & TEXT_DECORATION_STYLE_WAVY){
         double   amp  = vextent/10.0;
         double   x    = ps[Geom::X];
-        double   y    = ps[Geom::Y];
+        double   y    = ps[Geom::Y] + poff[Geom::Y];
         dc.moveTo(Geom::Point(x, y + amp * wave[i]));
         while(1){
            i = ((i + 1) & 15);
@@ -300,17 +331,25 @@ pf = Geom::Point(step * round(p2[Geom::X]/step),p2[Geom::Y]);
            dc.lineTo(Geom::Point(x, y + amp * wave[i]));
            if(x >= pf[Geom::X])break;
         }
-   }  
+        y = ps[Geom::Y] - poff[Geom::Y];
+        dc.lineTo(Geom::Point(x, y + amp * wave[i]));
+        while(1){
+           i = ((i - 1) & 15);
+           x -= step;
+           dc.lineTo(Geom::Point(x, y + amp * wave[i]));
+           if(x <= ps[Geom::X])break;
+        }
+        dc.closePath();
+    }  
     else { // TEXT_DECORATION_STYLE_SOLID, also default in case it was not set for some reason
-         dc.moveTo(ps);
-         dc.lineTo(pf);
-//         dc.revrectangle(Geom::Rect(ps,pf));
+        dc.rectangle( Geom::Rect(ps + poff, pf - poff));
     } 
 }
 
 /* returns scaled line thickness */
-double DrawingText::decorateItem(DrawingContext &dc, Geom::Affine const &aff, double phase_length)
+void DrawingText::decorateItem(DrawingContext &dc, double phase_length, bool under)
 {
+    if (_nrstyle.font_size < 1.0e-32)return;  // would cause a divide by zero and nothing would be visible anyway
     double tsp_width_adj                = _nrstyle.tspan_width                     / _nrstyle.font_size;
     double tsp_asc_adj                  = _nrstyle.ascender                        / _nrstyle.font_size;
     double tsp_size_adj                 = (_nrstyle.ascender + _nrstyle.descender) / _nrstyle.font_size;
@@ -318,49 +357,54 @@ double DrawingText::decorateItem(DrawingContext &dc, Geom::Affine const &aff, do
     double final_underline_thickness    = CLAMP(_nrstyle.underline_thickness,    tsp_size_adj/30.0, tsp_size_adj/10.0);
     double final_line_through_thickness = CLAMP(_nrstyle.line_through_thickness, tsp_size_adj/30.0, tsp_size_adj/10.0);
 
-    double scale = aff.descrim();
     double xphase = phase_length/ _nrstyle.font_size; // used to figure out phase of patterns
 
-    Inkscape::DrawingContext::Save save(dc);
-    dc.transform(aff);  // must be leftmost affine in span
-
     Geom::Point p1;
     Geom::Point p2;
     // All lines must be the same thickness, in combinations, line_through trumps underline
     double thickness = final_underline_thickness;
-    if(_nrstyle.text_decoration_line & TEXT_DECORATION_LINE_UNDERLINE){
-        p1 = Geom::Point(0.0,          -_nrstyle.underline_position);
-        p2 = Geom::Point(tsp_width_adj,-_nrstyle.underline_position);
-        decorateStyle(dc, tsp_size_adj, xphase, p1, p2);
-    }
-    if(_nrstyle.text_decoration_line & TEXT_DECORATION_LINE_OVERLINE){
-        p1 = Geom::Point(0.0,          tsp_asc_adj -_nrstyle.underline_position + 1 * final_underline_thickness);
-        p2 = Geom::Point(tsp_width_adj,tsp_asc_adj -_nrstyle.underline_position + 1 * final_underline_thickness);
-        decorateStyle(dc, tsp_size_adj, xphase,  p1, p2);
-    }
-    if(_nrstyle.text_decoration_line & TEXT_DECORATION_LINE_LINETHROUGH){
-        thickness = final_line_through_thickness;
-        p1 = Geom::Point(0.0,          _nrstyle.line_through_position);
-        p2 = Geom::Point(tsp_width_adj,_nrstyle.line_through_position);
-        decorateStyle(dc, tsp_size_adj, xphase,  p1, p2);
-    }
-    // Obviously this does not blink, but it does indicate which text has been set with that attribute
-    if(_nrstyle.text_decoration_line & TEXT_DECORATION_LINE_BLINK){
-        thickness = final_line_through_thickness;
-        p1 = Geom::Point(0.0,          _nrstyle.line_through_position - 2*final_line_through_thickness);
-        p2 = Geom::Point(tsp_width_adj,_nrstyle.line_through_position - 2*final_line_through_thickness);
-        decorateStyle(dc, tsp_size_adj, xphase,  p1, p2);
-        p1 = Geom::Point(0.0,          _nrstyle.line_through_position + 2*final_line_through_thickness);
-        p2 = Geom::Point(tsp_width_adj,_nrstyle.line_through_position + 2*final_line_through_thickness);
-        decorateStyle(dc, tsp_size_adj, xphase,  p1, p2);
+    dc.setTolerance(0.5); // Is this really necessary... could effect dots.
+
+    if( under ) {
+
+        if(_nrstyle.text_decoration_line & TEXT_DECORATION_LINE_UNDERLINE){
+            p1 = Geom::Point(0.0,          -_nrstyle.underline_position);
+            p2 = Geom::Point(tsp_width_adj,-_nrstyle.underline_position);
+            decorateStyle(dc, tsp_size_adj, xphase, p1, p2, thickness);
+        }
+
+        if(_nrstyle.text_decoration_line & TEXT_DECORATION_LINE_OVERLINE){
+            p1 = Geom::Point(0.0,          tsp_asc_adj -_nrstyle.underline_position + 1 * final_underline_thickness);
+            p2 = Geom::Point(tsp_width_adj,tsp_asc_adj -_nrstyle.underline_position + 1 * final_underline_thickness);
+            decorateStyle(dc, tsp_size_adj, xphase,  p1, p2, thickness);
+        }
+
+    } else {
+        // Over
+
+        if(_nrstyle.text_decoration_line & TEXT_DECORATION_LINE_LINETHROUGH){
+            thickness = final_line_through_thickness;
+            p1 = Geom::Point(0.0,          _nrstyle.line_through_position);
+            p2 = Geom::Point(tsp_width_adj,_nrstyle.line_through_position);
+            decorateStyle(dc, tsp_size_adj, xphase,  p1, p2, thickness);
+        }
+
+        // Obviously this does not blink, but it does indicate which text has been set with that attribute
+        if(_nrstyle.text_decoration_line & TEXT_DECORATION_LINE_BLINK){
+            thickness = final_line_through_thickness;
+            p1 = Geom::Point(0.0,          _nrstyle.line_through_position - 2*final_line_through_thickness);
+            p2 = Geom::Point(tsp_width_adj,_nrstyle.line_through_position - 2*final_line_through_thickness);
+            decorateStyle(dc, tsp_size_adj, xphase,  p1, p2, thickness);
+            p1 = Geom::Point(0.0,          _nrstyle.line_through_position + 2*final_line_through_thickness);
+            p2 = Geom::Point(tsp_width_adj,_nrstyle.line_through_position + 2*final_line_through_thickness);
+            decorateStyle(dc, tsp_size_adj, xphase,  p1, p2, thickness);
+        }
     }
-    thickness *= scale;
-    return(thickness);
 }
 
 unsigned DrawingText::_renderItem(DrawingContext &dc, Geom::IntRect const &/*area*/, unsigned /*flags*/, DrawingItem * /*stop_at*/)
 {
-   if (_drawing.outline()) {
+    if (_drawing.outline()) {
         guint32 rgba = _drawing.outlinecolor;
         Inkscape::DrawingContext::Save save(dc);
         dc.setSource(rgba);
@@ -382,124 +426,187 @@ unsigned DrawingText::_renderItem(DrawingContext &dc, Geom::IntRect const &/*are
         return RENDER_OK;
     }
 
-    // NOTE: this is very similar to drawing-shape.cpp; the only difference is in path feeding
-    double leftmost     = DBL_MAX;
-    double phase_length = 0.0;
-    bool   firsty       = true;
-    bool   decorate     = true;
-    double starty       = 0.0;
-    Geom::Affine aff;
-    using Geom::X;
-    using Geom::Y;
-
-    // NOTE:
+    // NOTE: This is very similar to drawing-shape.cpp; the only differences are in path feeding
+    // and in applying text decorations.
+
+
+    // Do we have text decorations?
+    bool decorate = (_nrstyle.text_decoration_line != TEXT_DECORATION_LINE_CLEAR );
+
     // prepareFill / prepareStroke need to be called with _ctm in effect.
     // However, we might need to apply a different ctm for glyphs.
     // Therefore, only apply this ctm temporarily.
-    bool has_stroke, has_fill;
+    bool has_stroke    = false;
+    bool has_fill      = false;
+    bool has_td_fill   = false;
+    bool has_td_stroke = false;
     {
         Inkscape::DrawingContext::Save save(dc);
         dc.transform(_ctm);
 
-        has_fill   = _nrstyle.prepareFill(  dc, _item_bbox);
-        has_stroke = _nrstyle.prepareStroke(dc, _item_bbox);
+        has_fill      = _nrstyle.prepareFill(                dc, _item_bbox);
+        has_stroke    = _nrstyle.prepareStroke(              dc, _item_bbox);
+
+        // Avoid creating patterns if not needed
+        if( decorate ) {
+            has_td_fill   = _nrstyle.prepareTextDecorationFill(  dc, _item_bbox);
+            has_td_stroke = _nrstyle.prepareTextDecorationStroke(dc, _item_bbox);
+        }
     }
 
-    if (has_fill || has_stroke) {
-        Geom::Affine rotinv;
-        bool invset = false;
+    if (has_fill || has_stroke || has_td_fill || has_td_stroke) {
 
-        // accumulate the path that represents the glyphs
-        for (ChildrenList::iterator i = _children.begin(); i != _children.end(); ++i) {
-            DrawingGlyphs *g = dynamic_cast<DrawingGlyphs *>(&*i);
-            if (!g) throw InvalidItemException();
-            if (!invset) {
-                rotinv = g->_ctm.withoutTranslation().inverse();
-                invset = true;
-            }
+        // Determine order for fill and stroke.
+        // Text doesn't have markers, we can do paint-order quick and dirty.
+        bool fill_first = false;
+        if( _nrstyle.paint_order_layer[0] == NRStyle::PAINT_ORDER_NORMAL ||
+            _nrstyle.paint_order_layer[0] == NRStyle::PAINT_ORDER_FILL   ||
+            _nrstyle.paint_order_layer[2] == NRStyle::PAINT_ORDER_STROKE ) {
+            fill_first = true;
+        } // Won't get "stroke fill stroke" but that isn't 'valid'
+
+
+        // Determine geometry of text decoration
+        double phase_length = 0.0;
+        Geom::Affine aff;
+        if( decorate ) {
+
+            Geom::Affine rotinv;
+            bool   invset    = false;
+            double leftmost  = DBL_MAX;
+            bool   first_y   = true;
+            double start_y   = 0.0;
+            for (ChildrenList::iterator i = _children.begin(); i != _children.end(); ++i) {
+
+                DrawingGlyphs *g = dynamic_cast<DrawingGlyphs *>(&*i);
+                if (!g) throw InvalidItemException();
+
+                if (!invset) {
+                    rotinv = g->_ctm.withoutTranslation().inverse();
+                    invset = true;
+                }
 
-            Inkscape::DrawingContext::Save save(dc);
-            if (g->_ctm.isSingular()) continue;
-            dc.transform(g->_ctm);
-            if (g->_drawable) {
-                dc.path(*g->_font->PathVector(g->_glyph));
-            }
-            // get the leftmost affine transform (leftmost defined with respect to the x axis of the first transform).  
-            // That way the decoration will work no matter what mix of L->R, R->L text is in the span.
-            if (_nrstyle.text_decoration_line != TEXT_DECORATION_LINE_CLEAR) {
                 Geom::Point pt = g->_ctm.translation() * rotinv;
-                if (pt[X] < leftmost) {
-                    leftmost     = pt[X];
+                if (pt[Geom::X] < leftmost) {
+                    leftmost     = pt[Geom::X];
                     aff          = g->_ctm;
                     phase_length = g->_pl;
                 }
-                /* If the text has been mapped onto a path, which causes y to vary, drop the text decorations.
-                   To handle that properly would need a conformal map
-                */
-                if (firsty) {
-                    firsty = false;
-                    starty = pt[Y];
+
+                // Check for text on a path. FIXME: This needs better test (and probably not here).
+                if (first_y) {
+                    first_y = false;
+                    start_y = pt[Geom::Y];
                 }
-                else if (fabs(pt[Y] - starty) > 1.0e-6) {
+                else if (fabs(pt[Geom::Y] - start_y) > 1.0e-6) {
+                    //  If the text has been mapped onto a path, which causes y to vary, drop the
+                    //  text decorations.  To handle that properly would need a conformal map.
                     decorate = false;
                 }
             }
         }
 
-        // draw the text itself
-        // we need to apply this object's ctm again
-        Inkscape::DrawingContext::Save save(dc);
-        dc.transform(_ctm);
+        // Draw text decorations that go UNDER the text (underline, over-line)
+        if( decorate ) {
 
-        // Text doesn't have markers, we can do paint-order quick and dirty.
-        bool fill_first = false;
-        if( _nrstyle.paint_order_layer[0] == NRStyle::PAINT_ORDER_NORMAL ||
-            _nrstyle.paint_order_layer[0] == NRStyle::PAINT_ORDER_FILL   ||
-            _nrstyle.paint_order_layer[2] == NRStyle::PAINT_ORDER_STROKE ) {
-            fill_first = true;
-        } // Won't get "stroke fill stroke" but that isn't 'valid'
+            {
+                Inkscape::DrawingContext::Save save(dc);
+                dc.transform(aff);  // must be leftmost affine in span
+                decorateItem(dc, phase_length, true);
+            }
+
+            {
+                Inkscape::DrawingContext::Save save(dc);
+                dc.transform(_ctm);  // Needed so that fill pattern rotates with text
+
+                if (has_td_fill && fill_first) {
+                    _nrstyle.applyTextDecorationFill(dc);
+                    dc.fillPreserve();
+                }
+
+                if (has_td_stroke) {
+                    _nrstyle.applyTextDecorationStroke(dc);
+                    dc.strokePreserve();
+                }
+
+                if (has_td_fill && !fill_first) {
+                    _nrstyle.applyTextDecorationFill(dc);
+                    dc.fillPreserve();
+                }
 
-        if (has_fill && fill_first) {
-            _nrstyle.applyFill(dc);
-            dc.fillPreserve();
+            }
+
+            dc.newPath(); // Clear text-decoration path
         }
 
-        if (has_stroke) {
-            _nrstyle.applyStroke(dc);
-            dc.strokePreserve();
+        // accumulate the path that represents the glyphs
+        for (ChildrenList::iterator i = _children.begin(); i != _children.end(); ++i) {
+            DrawingGlyphs *g = dynamic_cast<DrawingGlyphs *>(&*i);
+            if (!g) throw InvalidItemException();
+
+            Inkscape::DrawingContext::Save save(dc);
+            if (g->_ctm.isSingular()) continue;
+            dc.transform(g->_ctm);
+            if (g->_drawable) {
+                dc.path(*g->_font->PathVector(g->_glyph));
+            }
         }
 
-        if (has_fill && !fill_first) {
-            _nrstyle.applyFill(dc);
-            dc.fillPreserve();
+        // Draw the glyphs.
+        {
+            Inkscape::DrawingContext::Save save(dc);
+            dc.transform(_ctm);
+
+            if (has_fill && fill_first) {
+                _nrstyle.applyFill(dc);
+                dc.fillPreserve();
+            }
+
+            if (has_stroke) {
+                _nrstyle.applyStroke(dc);
+                dc.strokePreserve();
+            }
+
+            if (has_fill && !fill_first) {
+                _nrstyle.applyFill(dc);
+                dc.fillPreserve();
+            }
         }
+        dc.newPath(); // Clear glyphs path
+
+        // Draw text decorations that go OVER the text (line through, blink)
+        if (decorate) {
 
-        dc.newPath(); // clear path
-
-        // draw text decoration
-        if (_nrstyle.text_decoration_line != TEXT_DECORATION_LINE_CLEAR && decorate) {
-            guint32 ergba;
-            if (_nrstyle.text_decoration_useColor) { // color different from the glyph
-                ergba =  SP_RGBA32_F_COMPOSE(
-                    _nrstyle.text_decoration_color.color.v.c[0],
-                    _nrstyle.text_decoration_color.color.v.c[1],
-                    _nrstyle.text_decoration_color.color.v.c[2],
-                    1.0);
+            {
+                Inkscape::DrawingContext::Save save(dc);
+                dc.transform(aff);  // must be leftmost affine in span
+                decorateItem(dc, phase_length, false);
             }
-            else { // whatever the current fill color is
-                ergba =  SP_RGBA32_F_COMPOSE(
-                    _nrstyle.fill.color.v.c[0],
-                    _nrstyle.fill.color.v.c[1],
-                    _nrstyle.fill.color.v.c[2],
-                    1.0);
+
+            {
+                Inkscape::DrawingContext::Save save(dc);
+                dc.transform(_ctm);  // Needed so that fill pattern rotates with text
+                
+                if (has_td_fill && fill_first) {
+                    _nrstyle.applyTextDecorationFill(dc);
+                    dc.fillPreserve();
+                }
+
+                if (has_td_stroke) {
+                    _nrstyle.applyTextDecorationStroke(dc);
+                    dc.strokePreserve();
+                }
+
+                if (has_td_fill && !fill_first) {
+                    _nrstyle.applyTextDecorationFill(dc);
+                    dc.fillPreserve();
+                }
+
             }
-            dc.setSource(ergba);
-            dc.setTolerance(0.5);
-            double thickness = decorateItem(dc, aff, phase_length);
-            dc.setLineWidth(thickness);
-            dc.strokePreserve();
-            dc.newPath(); // clear path
+
+            dc.newPath(); // Clear text-decoration path
         }
+
     }
     return RENDER_OK;
 }
diff --git a/src/display/drawing-text.h b/src/display/drawing-text.h
index 41039d85d..4453a3db4 100644
--- a/src/display/drawing-text.h
+++ b/src/display/drawing-text.h
@@ -68,8 +68,8 @@ protected:
     virtual DrawingItem *_pickItem(Geom::Point const &p, double delta, unsigned flags);
     virtual bool _canClip();
 
-    double decorateItem(DrawingContext &dc, Geom::Affine const &aff, double phase_length);
-    void decorateStyle(DrawingContext &dc, double vextent, double xphase, Geom::Point const &p1, Geom::Point const &p2);
+    void decorateItem(DrawingContext &dc, double phase_length, bool under);
+    void decorateStyle(DrawingContext &dc, double vextent, double xphase, Geom::Point const &p1, Geom::Point const &p2, double thickness);
     NRStyle _nrstyle;
 
     friend class DrawingGlyphs;
diff --git a/src/display/nr-style.cpp b/src/display/nr-style.cpp
index 09a28e63c..ec3117079 100644
--- a/src/display/nr-style.cpp
+++ b/src/display/nr-style.cpp
@@ -54,8 +54,13 @@ NRStyle::NRStyle()
     , line_join(CAIRO_LINE_JOIN_MITER)
     , fill_pattern(NULL)
     , stroke_pattern(NULL)
+    , text_decoration_fill_pattern(NULL)
+    , text_decoration_stroke_pattern(NULL)
     , text_decoration_line(TEXT_DECORATION_LINE_CLEAR)
     , text_decoration_style(TEXT_DECORATION_STYLE_CLEAR)
+    , text_decoration_fill()
+    , text_decoration_stroke()
+    , text_decoration_stroke_width(0.0)
     , phase_length(0.0)
     , tspan_line_start(false)
     , tspan_line_end(false)
@@ -76,11 +81,15 @@ NRStyle::~NRStyle()
 {
     if (fill_pattern) cairo_pattern_destroy(fill_pattern);
     if (stroke_pattern) cairo_pattern_destroy(stroke_pattern);
+    if (text_decoration_fill_pattern) cairo_pattern_destroy(text_decoration_fill_pattern);
+    if (text_decoration_stroke_pattern) cairo_pattern_destroy(text_decoration_stroke_pattern);
     if (dash){
         delete [] dash;
     }
     fill.clear();
     stroke.clear();
+    text_decoration_fill.clear();
+    text_decoration_stroke.clear();
 }
 
 void NRStyle::set(SPStyle *style)
@@ -195,15 +204,65 @@ void NRStyle::set(SPStyle *style)
     if(style->text_decoration_style.dashed       ){ text_decoration_style |= TEXT_DECORATION_STYLE_DASHED   + TEXT_DECORATION_STYLE_SET; }
     if(style->text_decoration_style.wavy         ){ text_decoration_style |= TEXT_DECORATION_STYLE_WAVY     + TEXT_DECORATION_STYLE_SET; }
  
+    /* FIXME
+       The meaning of text-decoration-color in CSS3 for SVG is ambiguous (2014-05-06).  Set
+       it for fill, for stroke, for both?  Both would seem like the obvious choice but what happens
+       is that for text which is just fill (very common) it makes the lines fatter because it
+       enables stroke on the decorations when it wasn't present on the text.  That contradicts the
+       usual behavior where the text and decorations by default have the same fill/stroke.
+       
+       The behavior here is that if color is defined it is applied to text_decoration_fill/stroke
+       ONLY if the corresponding fill/stroke is also present.
+       
+       Hopefully the standard will be clarified to resolve this issue.
+    */
+
+    SPStyle* style_td = style;
+    if ( style->text_decoration.style_td ) style_td = style->text_decoration.style_td;
+    text_decoration_stroke.opacity = SP_SCALE24_TO_FLOAT(style_td->stroke_opacity.value);
+    text_decoration_stroke_width = style_td->stroke_width.computed;
+
     if( style->text_decoration_color.set          ||
         style->text_decoration_color.inherit      || 
-        style->text_decoration_color.currentcolor ){
-        text_decoration_color.set(style->text_decoration_color.value.color);
-        text_decoration_useColor = true;
-    }
-    else {
-        text_decoration_color.clear();
-        text_decoration_useColor = false;
+        style->text_decoration_color.currentcolor ) {
+
+        if(style->fill.isPaintserver() || style->fill.isColor()) {
+            // SVG sets color specifically
+            text_decoration_fill.set(style->text_decoration_color.value.color);
+        } else {
+            // No decoration fill because no text fill
+            text_decoration_fill.clear();
+        }
+
+        if(style->stroke.isPaintserver() || style->stroke.isColor()) {
+            // SVG sets color specifically
+            text_decoration_stroke.set(style->text_decoration_color.value.color);
+        } else {
+            // No decoration stroke because no text stroke
+            text_decoration_stroke.clear();
+        }
+
+    } else {
+        // Pick color/pattern from text
+        if ( style_td->fill.isPaintserver() ) {
+            text_decoration_fill.set(style_td->getFillPaintServer());
+        } else if ( style_td->fill.isColor() ) {
+            text_decoration_fill.set(style_td->fill.value.color);
+        } else if ( style_td->fill.isNone() ) {
+            text_decoration_fill.clear();
+        } else {
+            g_assert_not_reached();
+        }
+
+        if ( style_td->stroke.isPaintserver() ) {
+            text_decoration_stroke.set(style_td->getStrokePaintServer());
+        } else if ( style_td->stroke.isColor() ) {
+            text_decoration_stroke.set(style_td->stroke.value.color);
+        } else if ( style_td->stroke.isNone() ) {
+            text_decoration_stroke.clear();
+        } else {
+            g_assert_not_reached();
+        }
     }
 
     if(text_decoration_line != TEXT_DECORATION_LINE_CLEAR){
@@ -232,10 +291,8 @@ bool NRStyle::prepareFill(Inkscape::DrawingContext &dc, Geom::OptRect const &pai
     if (!fill_pattern) {
         switch (fill.type) {
         case PAINT_SERVER: {
-            //fill_pattern = sp_paint_server_create_pattern(fill.server, dc.raw(), paintbox, fill.opacity);
-        	fill_pattern = fill.server->pattern_new(dc.raw(), paintbox, fill.opacity);
-
-		} break;
+            fill_pattern = fill.server->pattern_new(dc.raw(), paintbox, fill.opacity);
+            } break;
         case PAINT_COLOR: {
             SPColor const &c = fill.color;
             fill_pattern = cairo_pattern_create_rgba(
@@ -254,14 +311,38 @@ void NRStyle::applyFill(Inkscape::DrawingContext &dc)
     dc.setFillRule(fill_rule);
 }
 
+bool NRStyle::prepareTextDecorationFill(Inkscape::DrawingContext &dc, Geom::OptRect const &paintbox)
+{
+    // update text decoration pattern
+    if (!text_decoration_fill_pattern) {
+        switch (text_decoration_fill.type) {
+        case PAINT_SERVER: {
+        	text_decoration_fill_pattern = text_decoration_fill.server->pattern_new(dc.raw(), paintbox, text_decoration_fill.opacity);
+		} break;
+        case PAINT_COLOR: {
+            SPColor const &c = text_decoration_fill.color;
+            text_decoration_fill_pattern = cairo_pattern_create_rgba(
+                c.v.c[0], c.v.c[1], c.v.c[2], text_decoration_fill.opacity);
+            } break;
+        default: break;
+        }
+    }
+    if (!text_decoration_fill_pattern) return false;
+    return true;
+}
+
+void NRStyle::applyTextDecorationFill(Inkscape::DrawingContext &dc)
+{
+    dc.setSource(text_decoration_fill_pattern);
+    // Fill rule does not matter, no intersections.
+}
+
 bool NRStyle::prepareStroke(Inkscape::DrawingContext &dc, Geom::OptRect const &paintbox)
 {
     if (!stroke_pattern) {
         switch (stroke.type) {
         case PAINT_SERVER: {
-            //stroke_pattern = sp_paint_server_create_pattern(stroke.server, dc.raw(), paintbox, stroke.opacity);
         	stroke_pattern = stroke.server->pattern_new(dc.raw(), paintbox, stroke.opacity);
-
 		} break;
         case PAINT_COLOR: {
             SPColor const &c = stroke.color;
@@ -285,13 +366,46 @@ void NRStyle::applyStroke(Inkscape::DrawingContext &dc)
     cairo_set_dash(dc.raw(), dash, n_dash, dash_offset); // fixme
 }
 
+bool NRStyle::prepareTextDecorationStroke(Inkscape::DrawingContext &dc, Geom::OptRect const &paintbox)
+{
+    if (!text_decoration_stroke_pattern) {
+        switch (text_decoration_stroke.type) {
+        case PAINT_SERVER: {
+        	text_decoration_stroke_pattern = text_decoration_stroke.server->pattern_new(dc.raw(), paintbox, text_decoration_stroke.opacity);
+		} break;
+        case PAINT_COLOR: {
+            SPColor const &c = text_decoration_stroke.color;
+            text_decoration_stroke_pattern = cairo_pattern_create_rgba(
+                c.v.c[0], c.v.c[1], c.v.c[2], text_decoration_stroke.opacity);
+            } break;
+        default: break;
+        }
+    }
+    if (!text_decoration_stroke_pattern) return false;
+    return true;
+}
+
+void NRStyle::applyTextDecorationStroke(Inkscape::DrawingContext &dc)
+{
+    dc.setSource(text_decoration_stroke_pattern);
+    dc.setLineWidth(text_decoration_stroke_width);
+    dc.setLineCap(CAIRO_LINE_CAP_BUTT);
+    dc.setLineJoin(CAIRO_LINE_JOIN_MITER);
+    dc.setMiterLimit(miter_limit);
+    cairo_set_dash(dc.raw(), 0, 0, 0.0); // fixme (no dash)
+}
+
 void NRStyle::update()
 {
     // force pattern update
     if (fill_pattern) cairo_pattern_destroy(fill_pattern);
     if (stroke_pattern) cairo_pattern_destroy(stroke_pattern);
+    if (text_decoration_fill_pattern) cairo_pattern_destroy(text_decoration_fill_pattern);
+    if (text_decoration_stroke_pattern) cairo_pattern_destroy(text_decoration_stroke_pattern);
     fill_pattern = NULL;
     stroke_pattern = NULL;
+    text_decoration_fill_pattern = NULL;
+    text_decoration_stroke_pattern = NULL;
 }
 
 /*
diff --git a/src/display/nr-style.h b/src/display/nr-style.h
index ca880c00b..83bcb1ab7 100644
--- a/src/display/nr-style.h
+++ b/src/display/nr-style.h
@@ -30,8 +30,12 @@ struct NRStyle {
     void set(SPStyle *);
     bool prepareFill(Inkscape::DrawingContext &dc, Geom::OptRect const &paintbox);
     bool prepareStroke(Inkscape::DrawingContext &dc, Geom::OptRect const &paintbox);
+    bool prepareTextDecorationFill(Inkscape::DrawingContext &dc, Geom::OptRect const &paintbox);
+    bool prepareTextDecorationStroke(Inkscape::DrawingContext &dc, Geom::OptRect const &paintbox);
     void applyFill(Inkscape::DrawingContext &dc);
     void applyStroke(Inkscape::DrawingContext &dc);
+    void applyTextDecorationFill(Inkscape::DrawingContext &dc);
+    void applyTextDecorationStroke(Inkscape::DrawingContext &dc);
     void update();
 
     enum PaintType {
@@ -67,6 +71,8 @@ struct NRStyle {
 
     cairo_pattern_t *fill_pattern;
     cairo_pattern_t *stroke_pattern;
+    cairo_pattern_t *text_decoration_fill_pattern;
+    cairo_pattern_t *text_decoration_stroke_pattern;
 
     enum PaintOrderType {
         PAINT_ORDER_NORMAL,
@@ -97,8 +103,9 @@ struct NRStyle {
 
     int   text_decoration_line;
     int   text_decoration_style;
-    Paint text_decoration_color;
-    bool  text_decoration_useColor;      // if false, use whatever the glyph color was
+    Paint text_decoration_fill;
+    Paint text_decoration_stroke;
+    float text_decoration_stroke_width;
     // These are the same as in style.h
     float phase_length;
     bool  tspan_line_start;