Text decoration dialog, first pass

Bug #1269206 reported by David Mathog
34
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Inkscape
In Progress
Wishlist
Unassigned

Bug Description

The attached patch implements a first pass of modifications to the text and font dialog that let it control text decorations.
Shift-control-t opens that dialog, select the "decorate" tab.

This patch also includes the changes in the patch in bug 1243401, which restored text decoration rendering.

This patch is applied in the typical manner, but it modifies share/icons/icons.svg, and those changes will not be evident until
that file is copied to wherever you keep your system's version of that file. On my linux box it was: /usr/local/share/inkscape/icons/icons.svg.

SVG 1.1 only supports CSS 2 text decorations (line type). Future SVG will probably support CSS3 (line style and color too).
There is a switch in the tab to select CSS 2 or CSS 3 compatibility. SInce this is a primitive first pass, the color and style lines do not gray out when CSS 2 is selected. Additionally the color support is rudimentary at this point for CSS 3, it only lets you choose whether the color comes from the text's fill or if it is "custom", but custom at this point is just a fixed color. To be useful that needs to open up into a color picker. Additionally the decorations buttons do not yet pick up their state from the selected text.

A test file and a screen shot of it will be posted next.

There is one outright bug in this somewhere. Every once and a while the "text-decoration" field is placed outside the style, like this:

style="...."
text-decoration="underline"

whereas it should be

style="...; text-decoration:underline;"

If one of those outside the style fields is present the "?" inherit button will generally not work properly.

---
Related reports:
Bug #172133 “Implement SVG text-decoration - underline, strikethrough, etc.”
Bug #1243401 “Clickable area of a text object is much larger than the object itself”

Tags: css styles text
Revision history for this message
David Mathog (mathog) wrote :
Revision history for this message
David Mathog (mathog) wrote :
Revision history for this message
David Mathog (mathog) wrote :
Revision history for this message
David Mathog (mathog) wrote :
Revision history for this message
ScislaC (scislac) wrote :

On Ubuntu 13.10 with the patch applied to trunk, it compiles, however crashes on running.

ERROR:style.cpp:481:void sp_style_paint_server_ref_modified(SPObject*, guint, SPStyle*): code should not be reached

Program received signal SIGABRT, Aborted.
0x00007fffefe8af77 in __GI_raise (sig=sig@entry=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.

Revision history for this message
David Mathog (mathog) wrote :

I cannot reproduce that! That is in code I didn't modify, although most likely code I did modify calls it (eventually).

Please help me figure out what the key differences are between our systems and inkscape directories.

For inkscape I did this clean(ish) test:

mv ~/.config/inkscape ~/.config/old_inkscape # the only place I know of that holds settings
bzr revert
bzr update # brought it to revision 12395
patch -p0 < changes_2014_01_14a_small.patch
cp /usr/local/src/inkscape_trunk/share/icons/icons.svg /usr/local/share/inkscape/icons/icons.svg
make
src/inkscape

and it started without any problems.

My system is:
Ubuntu 12.04.3 LTS
32 bits
gcc --version
gcc-4.6.real (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Revision history for this message
ScislaC (scislac) wrote :

Interestingly it appears to be caused by the symbolic icons file (which is in our repo "share/icons/symbolic_icons.svg"). When not using that it runs fine.

Aside from that crash, any feedback I would have are things you already mentioned in the initial message accompanying the patch. Nice to see the UI for this coming together.

Revision history for this message
ScislaC (scislac) wrote :

I take that back... one other comment. I'm not sure if you are aware or did it, but check out http://wiki.inkscape.org/wiki/index.php/Icons#Single_SVG_File for a customization to your preferences file for when you edit the icons files (to reduce file size).

Revision history for this message
su_v (suv-lp) wrote :

Crash reproduced on OS X 10.7.5, using a custom icons.svg file in ~/.config/inkscape/icons (based on the symbolic_icons.svg file), as well as with a copy of the tango_icons.svg file which is distributed with Inkscape.
Crash not reproduced with yet another custom icon file (not distributed with Inkscape).

Revision history for this message
David Mathog (mathog) wrote :

Was the icons.svg file from the first patch alone enough to cause the crash, or did the code also need to be modified in order to trigger the crash? Also, was there a problem loading the new icons.svg into inkscape (with open) too?

In any case, for now, attached is a newer patch with the same code changes but a different icons.svg file. I made this one using the preferences setting ScislaC indicated, and then hand edited the final icons.svg so that it only differed from the original by the lines corresponding to the new icons. I did try using "clean document" and "save" but it made a lot of unrelated changes nowhere near the section I had just created, and those changes may be a separate issue, which is why I edited them all out, restoring the original.

Revision history for this message
su_v (suv-lp) wrote :

On 2014-01-16 20:10 +0100, David Mathog wrote:
> Was the icons.svg file from the first patch alone enough to cause the
> crash, or did the code also need to be modified in order to trigger the
> crash? Also, was there a problem loading the new icons.svg into
> inkscape (with open) too?

No. AFAICT the crash was not caused by your modified version of the shared icons.svg file. The crash occurs when loading certain files, among them apparently two alternative icons.svg files included in Inkscape's repo.

To reproduce the crash:
1) launch current trunk with your latest patch applied
2) use 'File > Open' and browse to 'share/icons' in the local branch
3) open 'tango_icons.svg' or 'symbolic_icons.svg'

--> Inkscape crashes (same backtrace) when attempting to open those files, be it while loading as (user) resource for the GUI icons or while loading for editing.

Revision history for this message
David Mathog (mathog) wrote :

 I can reproduce the crash for tango_icons.svg. Will from there. Thanks.

su_v (suv-lp)
tags: added: css styles text
Changed in inkscape:
importance: Undecided → Wishlist
status: New → In Progress
description: updated
Revision history for this message
David Mathog (mathog) wrote :
Download full text (3.3 KiB)

This patch should fix the crashes.

It adds the "dotted" style, which I forgot to put into the dialog and the icons. (That is CSS 3)

It also implements some feedback on selections into the dialog, so some of the buttons are set based on what has been selected. The CSS 2 and CSS 3 buttons are not set, basically because there is no consistent way to figure out which mode is in use. The color buttons are not set because that part of the dialog is still very rudimentary. Selection of a text region that uses purely inherited line,style,color does not at present set any of the buttons.

A couple of notes on text decorations, just to get them down somewhere:

1. CSS 3 is there to play around with, but I would not suggest using it for anything that needs to be portable. I don't think any other SVG renderer can use those fields. (Which is too bad, because dotted, dashed, and wavy decorations are common
in formatted text.)

2. The SVG spec says that text decorations are supposed to be drawn like the text, taking both fill and stroke from the corresponding text. At present the text decoration does not do that, it only works with a single color, not patterns and such. So what it does is first try to take the color from stroke, and if that fails, it takes the fill. (Unless CSS 3 is used, in which case the color can be set directly.) That results in text decorations that match (to my eye) the dominant color of the text. I think it is going to be a big mess when they try to get CSS 3 into SVG because that does allow specification of ONE color, but SVG according to the spec needs two for decorations.

3. The spec is awfully vague about how to draw the text decorations. Since it doesn't say how thick the line is supposed to be, if stroke was used to outline the decoration it could look like, well, anything. So I don't think SVG text decorations are going to look very similar from one application to another.

4. My goal at present is to get text_decorations working right for EMF input/output. That seems to work OK now. It should be possible to get output for most types. Input will be hard for any graphic file type that doesn't explicitly handle decorations. PDF, for instance, seems to just draw lines and doesn't indicate how they relate to the text.

5. It is possible with CSS 2 to set the text decoration color different from the text, but it takes two levels of <tspan> to do so. For instance, in level 1's style have: "fill:#FF0000; text-decoration:underline", but no text, and in level 2's style have:
fill:#0000FF, omit text-decoration (so it will inherit), and specify some text. Doing that through the dialog is a pain. The simplest way I have found is to enter AmessageA and set the color and decoration, then select "message" and change its color. Then edit off the A's on the end. With CSS 3 one could in theory specify the color directly, although the dialog's color capabilities are at present too rudimentary to allow that.

6. Text decorations other than strike-through are supposed to be drawn before the text, and strike-through after. At present they are all drawn after. The reason is that drawing anything first was mangli...

Read more...

Revision history for this message
David Mathog (mathog) wrote :

This is an example for testing text decorations. It has all the sodipodi stuff removed, so that it can be fed into web browsers. Inkscape does a terrible job selecting text in this example, it cannot figure out how to navigate to subsequent <tspan>s in a <text>. In any case, it has fill only, stroke only, and fill and stroke, so you can see some of the issues concerning text decoration draws with two colors.

Revision history for this message
David Mathog (mathog) wrote :

The attached patch takes the text decorations into full compliance (I think) with SVG 1.1, while still supporting
SVG 2's CSS 3. Changes:

1. It draws underline and overline underneath the text, strike-through over it.
2. It supports patterns in the fill and stroke. For instance, the letters can be "packed circles" with a red boundary, and the decorations will be that way too.
3. Added "dotted" to the text and font dialog (for CSS 3).
4. Fixed a glitch where occasionally a CSS 3 "text-decoration" would have the extra style value, but without color.

This patch includes all of the dc->ct changes from bug #1272073. (That is a partial step in making the text decoration code reusable for "render", as in going to PDF through Cairo.)

Known issues:

1. The font and text dialog still does not always pick up the right font size for a selected region of text. Consequently when the text decorations are added it sometimes unexpectedly changes the font. (Usually the size, sometimes the face.)
That seems to be a (pre-existing) problem in the method it uses to retrieve these values.
2. CSS 3 allows the "color" to be set, but there are both "fill" and "stroke". So when text-decoration-color or the longer text-decoration tags are used they only set "fill".
3. The dash property is not supported for stroke on text decorations. It was omitted because there are all sorts of problems getting the dashes to phase out properly when there are multiple tspans within one text. Bottom line, using dashed strokes looked dreadful when I was experimenting with them, so I did not implement it.

Suspected issues:

1. fill and stroke are set pervasively on objects and text-decoration-fill and text-decoration-stroke are derived from them. The current implementation has these new fields for all "style" objects, 99.9% of them with all text-decoration values clear, which is probably wasting a fair amount of memory since only text objects can use these fields. It would be more space efficient to bundle all of these optional pieces into a data structure accessed via a pointer, so the overhead would only be one NULL pointer per object.

Will post some examples tomorrow, after bug 1273900 is resolved, which at the moment is keeping me from rebuilding at the current trunk revision.

Revision history for this message
David Mathog (mathog) wrote :

Examples of more complex text decorations.

Top line: outer letters, have fill = "packed circle".
Bottom line: shows under/over drawing of text decorations.

The screen snap of the SVG part, as seen in Inkscape, is included as an image within the SVG.

The dotted line on the top center over "CDE" is an SVG 2/CSS 3 thing. If you view this example SVG in a browser that text-decoration command will be ignored and instead a solid overline will be seen. The color of the overline should not change, but depending on the browser, it might.

Revision history for this message
David Mathog (mathog) wrote :

This is the latest patch replacing the preceding ones. It fixes a couple of cases which were omitted having to do with opacity.

Discovered one bug - if text decoration uses a patterned stroke then selecting the object crashes Inkscape. More on that in a moment.

Revision history for this message
David Mathog (mathog) wrote :

This example contains more text decorations, including gradients on both text decoration fill and text decoration stroke

Revision history for this message
David Mathog (mathog) wrote :

This example can be used to demonstrate the problem when patterned strokes are used. If that object (the top one) is selected and then "copy", Inkscape crashes. It goes down in sp_style_paint_server_ref_modified in style.cpp, at line 476. Apparently when the object is copied the server name gets out of sync with that in stroke (and text_decoration_stroke), consequently none of the 4 test cases match, and it falls through to the assert where it crashes the program.

Oddly, this does not happen for the test case where only a pattern text_decoration_fill is used (middle text case).

The problem is present for gradients in stroke as well, but not for gradients in fill. (Not shown)

Revision history for this message
su_v (suv-lp) wrote :

Updated version of 'changes_2014_01_31a_textdecorateonly.patch' against trunk r13012 (patch from bug #1272073 has been committed to trunk) - please verify.

Revision history for this message
David Mathog (mathog) wrote :

That was close. Here is one made from revision 13018. It incorporates a couple of other small changes made in the interval (1: clean up some memory that was previously left dangling, 2: remove a commented out section of debugging code).

Revision history for this message
David Mathog (mathog) wrote :

The preceding patch had some problems applying at r13047. This one has the same changes, after hand mergingaround some of the incompatible changes to trunk in the interim.

Revision history for this message
David Mathog (mathog) wrote :

changes_2014_02_21b.patch replaces the preceding. This one also contains changes that fix the crash in message 19, above.

Note that copy/paste of text decorations having gradients is broken, but that is due to bug #1283193. Not sure how long that problem has been around because the crash on "copy" was preventing me from getting far enough to see it.

This patch has been tested on Linux in valgrind. Sadly the test was not very informative regarding possible memory problems because styles.cpp already has a very large number of entries in the valgrind output.

This patch has been tested on Windows XP and Linux.

Revision history for this message
David Mathog (mathog) wrote :

Oops, wrong patch! Sorry. The preceding one had a few other odds and ends in it. This one is only the text decoration dialog pieces.

Revision history for this message
su_v (suv-lp) wrote :

@David - JFYI: build failure with trunk r13126 + 'changes_2014_02_21b_textdecorateonly.patch' when using CPPFLAGS="-DWITH_SVG2" [1]:

   CXX display/drawing-shape.o
../../src/display/drawing-shape.cpp:159:43: error: no viable conversion from 'Inkscape::DrawingContext' to 'cairo_t *' (aka '_cairo *')
    bool has_fill = _nrstyle.prepareFill(dc, _item_bbox);
                                          ^~
../../src/display/nr-style.h:31:31: note: passing argument to parameter 'ct' here
    bool prepareFill(cairo_t *ct, Geom::OptRect const &paintbox);
                              ^
../../src/display/drawing-shape.cpp:163:28: error: no viable conversion from 'Inkscape::DrawingContext' to 'cairo_t *' (aka '_cairo *')
        _nrstyle.applyFill(dc);
                           ^~
../../src/display/nr-style.h:35:29: note: passing argument to parameter 'ct' here
    void applyFill(cairo_t *ct);
                            ^
../../src/display/drawing-shape.cpp:175:46: error: no viable conversion from 'Inkscape::DrawingContext' to 'cairo_t *' (aka '_cairo *')
    bool has_stroke = _nrstyle.prepareStroke(dc, _item_bbox);
                                             ^~
../../src/display/nr-style.h:32:33: note: passing argument to parameter 'ct' here
    bool prepareStroke(cairo_t *ct, Geom::OptRect const &paintbox);
                                ^
../../src/display/drawing-shape.cpp:181:30: error: no viable conversion from 'Inkscape::DrawingContext' to 'cairo_t *' (aka '_cairo *')
        _nrstyle.applyStroke(dc);
                             ^~
../../src/display/nr-style.h:36:31: note: passing argument to parameter 'ct' here
    void applyStroke(cairo_t *ct);
                              ^
1 warning and 4 errors generated.
make[3]: *** [display/drawing-shape.o] Error 1
  CXX display/drawing-text.o
../../src/display/drawing-text.cpp:536:36: error: no viable conversion from 'Inkscape::DrawingContext' to 'cairo_t *' (aka '_cairo *')
                _nrstyle.applyFill(dc);
                                   ^~
../../src/display/nr-style.h:35:29: note: passing argument to parameter 'ct' here
    void applyFill(cairo_t *ct);
                            ^
1 warning and 1 error generated.
make[3]: *** [display/drawing-text.o] Error 1
make[3]: Target `all-am' not remade because of errors.
make[2]: *** [all] Error 2

[1] to test the latest paint-order feature implemented by Tav, and described here: <http://tavmjong.free.fr/blog/?p=1090>

Revision history for this message
David Mathog (mathog) wrote :

Probably the #ifdef replicated those chunks of code, so applying the patch would then only hit one of the replicas. The patch changes all of those dc to dc.raw, so for sure the problem lines were not patched. Will try to look into it further later today.

Revision history for this message
David Mathog (mathog) wrote :

This patch (also) handles the -DWITH_SVG2 issue. (There does not seem to be a configure item that specifically enables that, just the generic CPPFLAGS mechanism.)

Note that for some reason or other Inkscape r13132 was crashing in an unrelated place after this patch was applied, until a full rebuild was forced with:

export CPPFLAGS="-DWITH_SVG2"
./configure --disable-strict-build
make clean
make

Revision history for this message
David Mathog (mathog) wrote :

for r13135. Some pieces moved from style.h to style-internal.h so the preceding patch broke.

Also includes the patch from bug #1291093, because if that bug remains multiple text decoration changes on multiline text result in the "apply" button on the "text decoration" tab migrating further and further down the screen.

Also includes a fix to preceding versions of this modification that were causing the Text and Font dialog to not pick up font line spacing and size properly on selected text when text decorations were absent.

Revision history for this message
David Mathog (mathog) wrote :

bringing latest patch over from #172133, where it was mistakenly posted.

Revision history for this message
pranav (pranav-d) wrote :

hi there,
i am using centos 6, using inkscape 0.49 version. I have svg with the following and it with underline object. while I am trying to convert into with command inkscape -z product_1.svg -e product_1.png it is not generating underline fonts.

any help. attached for the sample file.

Revision history for this message
David Mathog (mathog) wrote :

PNG output goes through Cairo and that bit of code does not currently do text decorations. This is true whether you use the command line or the GUI to create the PNG. PDF output goes through the same path, and you will observe that it too drops the text decorations. The EMF and WMF outputs retain this information because those conversions do not go through Cairo at all, basically they are a traverse the object tree, drawing as it goes.

So if you are between a rock and a hard place and need to get some document out in PNG format ASAP, save to EMF, then use one of the many programs (on Windows) that can read that, and save from there to PNG. (Even then, only the simplest text decorations will work, EMF doesn't support all the options in CSS.)

If you are want to fix this limitation, here is a brief overview of what is going on. The code of interest is mostly in

  src/extensions/internal

except for the code I added to draw text decorations on the screen, which is in src/display/drawing-text.cpp. I verified with the debugger that execution doesn't go to drawing-text.cpp on a PNG save. Instead in cairo-renderer.cpp it follows this path

CairoRenderer::renderItem to
sp_item_invoke_render to
sp_text_render to
text->layout.showGlyphs(ctx);

There is nothing but showGlyphs() in sp_text_render, which explains why the text decorations
disappear.

The draw to the screen and the draw to PNG seem to be a lot of apples and oranges, even though they both use Cairo. The screen drawing text operates on a DrawingContext and has access to an _nrstyle, whereas the Cairo section operates on a CairoRenderContext and an SPText. SPStyle can be retrieved from the SPText and the parts stored in _nrstyle obtained that way, but I'm not sure what the relationship is between DrawingContext and CairoRenderContext. So forcing sp_text_render to call DrawingText::_renderItem() doesn't look like a walk in the park.

Maybe it has been added since, but at the time Cairo didn't have any native functions specifically for drawing text decorations, these had to be built up out of lower level primitives. You can see how complicated that gets by looking around in drawing-text.cpp.

Revision history for this message
David Mathog (mathog) wrote :

s/you are want/you want/

To post a comment you must log in.