Extension for 0.92.x to fix line spacing in legacy documents

[su_v]

Text layout in upcoming Inkscape 0.92 will have better support for the CSS standard:
http://wiki.inkscape.org/wiki/index.php/Release_notes/0.92#Line_Spacing
These internal changes may cause differences in layout (line spacing) of text in legacy documents created with older versions of Inkscape.

The proposed extension can help easing the transition to 0.92 for users:
Source code:
https://gitlab.com/su-v/inx-legacytext
Available downloads:
https://gitlab.com/su-v/inx-legacytext/tags

The extension installs (for now) as 'Extensions > Document > Fix Legacy Text' and will process recursively all text objects in the selection or in the whole document (if nothing is selected), adjusting the outer and inner style properties of the text objects as needed.

Feedback (code-review) as well as problematic test cases (Inkscape SVG files) are most welcome. Discussion about the underlying internal changes themselves in Inkscape 0.92.x and trunk are off-topic in this report.

=====
Related reports:
Bug #1661281 “object disappears with v0.92”
Bug #1655483 “Inkscape 0.92 line space problem with text of Inkscape 0.91”
Bug #1655412 “Space between base lines misbehavior”
Bug #1652006 “Line spacing differs between 0.91 an 0.92b4”
Bug #1644299 “text and image not displaying in 0.92pre3”
Bug #1642133 “Inkscape shows text with ridiculously wide line spacing”
Bug #1617692 “Spacing Between Baselines Messed Up 0.92 15081”
Bug #1556400 “line height all equal in development version”

[Hachmann (marenhachmann)]

This is most welcome, suv! I'll try it on one of my Kielux Flyers and will report back.

[su_v (suv-lp)]

@Hachmann - thanks, that would be great. Please note that the code logic of the current fix for blank lines is flawed (I'm working on a better solution), and will ultimately depend on internal changes to preserve the space character (as mentioned in the README file).

[Hachmann (marenhachmann)]

So, these are the results so far with my Kielux documents from this year (tested with r15273, and extension version 0030a909):

1. While I this isn't a file that exposes the line-height issue, I got a crash when using the extension (without a reason for doing so) on the attached file (Linuxhotel-Gutschein_2016.svg). One time, it happened when using the extension, after having made a couple of modifications, one time it happened upon undo (with emergency save). The order is: Run extension, edit, undo edit, undo extension, crash.

"** (inkscape:28693): CRITICAL **: void SPObject::requestModified(unsigned int): assertion 'this->document != NULL' failed
Speicherzugriffsfehler"

In that same file, the extension also inserts a superfluous(?) period above the 'dieses'.

(1a. Btw. is it to be expected that I get an 'update document' in the history, and a changed file indicator asterisk in the window title, without having seen any popup about dpi changes? No changes in the svg tag in the XML editor.)

2. In a file where had manually changed line-height, the extension fixed the too-large line-height. It took quite long (>60 seconds, with remarkable processor load), although the problem only (visibly) occurred in one of the (many) texts. In the other files, it was a lot faster, even if they contained a similar number of texts. But as it's a one-time thing, this isn't really an issue :) Undo takes just as long, so I guess it made a lot of invisible changes. Please let me know if you'd also like to have that file (reminder for myself, in case: Programmflyer_Aushang_2016).

3. In the file 'Programm_Flyer_Entwurf_2016', the extension inserts really many full-stops (at the bottom right, 'Aussteller', but also at 'Öffnungszeiten'), although there aren't any blank lines. Are they all justified?

[su_v (suv-lp)]

On 2016-12-23 22:52 (+0100), Hachmann wrote:
> In that same file, the extension also inserts a superfluous(?) period
> above the 'dieses'.

There is an empty flowed text object in the document near that position
(id="flowRoot23315") ... i.e. a blank line (at least with the simple
logic the extension currently uses). I'll try how much effort it is to
detect and ignore such objects (there will be plenty of them around in
many Inkscape files out there - Inkscape does not auto-clean flowed text
elements if a text frame is drawn, but no text entered subsequently).

> (1a. Btw. is it to be expected that I get an 'update document' in the
> history, and a changed file indicator asterisk in the window title,
> without having seen any popup about dpi changes? No changes in the svg
> tag in the XML editor.)

Unrelated to this extension - 'Update Document' is something that is
done internally by inkscape itself [1]: it might update legacy guides
(adding new status for locking guides) or grids (internal change from
absolute units to user units) or perspective definitions (in case there
are 3D boxes used in the document).

> 2. In a file (...)

Please attach.

> 3. In the file 'Programm_Flyer_Entwurf_2016', the extension inserts
> really many full-stops (at the bottom right, 'Aussteller', but also at
> 'Öffnungszeiten'), although there aren't any blank lines. Are they all
> justified?

As commented earlier (comment 2), there is a logic flaw in the current
extension code to "fix blank lines"; and as mentioned in the README file
in the gitlab repo there is an Inkscape bug which prevents that the
intended workaround for legacy files to insert a space (instead of the
'.' placeholder character) can be used at the moment. I'll be happy to
use your file as test case once I have rewritten the fix_blank_line()
function in the extension to improve the detection of which tspans could
be (or had been) intended to act as blank lines, and how large the
computed font size of the line above is (that's the trickier part).

[1]:
https://bazaar.launchpad.net/~inkscape.dev/inkscape/0.92.x/view/head:/src/file.cpp#L722

[Hachmann (marenhachmann)]

Interesting, now I cannot reproduce that awfully long running of the extension (I did test it twice before... - same with undo. Weird...). Sorry... Attaching anyway, but it's probably not worth your time.

But I noticed that there is some text missing in 0.92, in the ...Entwurf file, no matter if I run the extension or not. It's at the top left in the table, next to the blackboard. It seems to be a tiny bit larger than before, so it falls out of the flowed text's text box.

Thank you for all the comments and explanations!

[su_v (suv-lp)]

On 2016-12-24 24:48 (+0100), Hachmann wrote:
> Interesting, now I cannot reproduce that awfully long running of the
> extension (I did test it twice before... - same with undo. Weird...).
> Sorry... Attaching anyway, but it's probably not worth your time.

You possibly had the 'Document Properties' dialog window opened at least
once in that session before running the extension? That dialog can
unfortunately cause such symptoms (slowing down replacing the content of
the current document with the one passed back from the extension script)
- they last during the rest of the session (save, quit Inkscape and
relaunch it if that happens as workaround). The doc prop dialog will
cause console messages like this in such situations:
** (inkscape:65009): WARNING **: Incompatible units

[Hachmann (marenhachmann)]

It's possible, I remember having looked up one file's dimensions in the dialog.
I didn't scroll up the whole console output, so this will be something to look for next time.

(just tested - it was still at least 3 times faster now than before, but noticeably slower than without having the dialog opened in the session. Anyway, it's unrelated to this specific extension, which seems to work quite well already. But it's good to know, in case a question about this pops up somewhere, and in case I get impatient ;-), so thank you, again!)

[su_v (suv-lp)]

Update pushed to gitlab; there is also a new branch 'dialog' in the repo to test (feedback welcome): it offers a dialog which allows to uncheck individual types of "fixes", as well as to visually "debug" blank lines by inserting a solid block instead of a no-break space.

[su_v (suv-lp)]

AFAIU it's too late to propose a new extension (or any other (internal) solution wrt the topic of text layout in legacy files as described) to be included in the upcoming release. I'll keep working on improving what is there in the repo, but won't further update the details in the report here at launchpad. If someone of the bug team thinks this should be considered for a bug-fix release (if one is actually happening), feel free to add a milestone (otherwise, I'll probably close the report after the final release is out).

[Hachmann (marenhachmann)]

Please, V, if this isn't accepted into 0.92, could you upload the extension to inkscape.org, or add it as an extension there and link to the download for the gitlab repo? It will be found more easily that way, and people will need it.

[su_v (suv-lp)]

@Hachmann - would you mind to test if such a format for download would be ok?
https://gitlab.com/su-v/inx-legacytext/tags/v0.0

[ The manually created and uploaded ZIP archive for the release tag (v0.0) only contains user-relevant files. My repositories for inkscape extensions usually also include some files for pylint and pep8 which are not needed to just install and use the extension, and are likely confusing to regular users. ]

[Hachmann (marenhachmann)]

Wish I had your automation abilities :) Yes, this looks perfect.

[jazzynico (jazzynico)]

Targeting to 0.92.1. Let me know if you think it's too early.

[su_v (suv-lp)]

@JazzyNico - I'm running out of test cases. The current state of the extension (v0.2) seems stable to me, I'm open though for any reviews, problematic test cases (Inkscape SVG files) and other feedback, here (bug #1652340) or as GitLab issues. Known limitations are described in the README file in the GitLab repo.

With regard to milestone 0.92.1: the extension currently has intentionally no strings marked for translation (English text in GUI labels and tooltips, no translatable error messages from the script), except for:

      <submenu _name="Document" />

This submenu in 'Extensions' is already used by the dpiswitcher extensions, which are part of 0.92, and the menu label likely exists as translated string for most of the languages already.

[su_v (suv-lp)]

Retracting the proposal to include the extension in Inkscape (0.92.x) and closing the report. @bug team - please remove the milestone.

The extension will be maintained in the gitlab repo as before - feel free to open issues there if specific to the extension.

[Hachmann (marenhachmann)]

I hope this is then going to be fixed differently, as was suggested by Marc on the mailing list?

[su_v (suv-lp)]

@Hachmann - yes. Details will probably depend on what is considered feasible for the 0.92.x series, as will hopefully be discussed based on Marc's proposal on the mailing list:
https://sourceforge.net/p/inkscape/mailman/message/35616959/

Further context:
https://bugs.launchpad.net/inkscape/+bug/1655483/comments/13

[Hachmann (marenhachmann)]

@su_v: Thank you!

[su_v (suv-lp)]

JFTR - initial internal implementation to fix baseline spacing of texts in legacy files on load has been committed to the stable release branch <lp:inkscape/0.92.x>:
* Revision 15338: merge 092+lineheight-fix branch
http://bazaar.launchpad.net/~inkscape.dev/inkscape/0.92.x/revision/15338

[Hachmann (marenhachmann)]

And even before Bryce's hard freeze, so it'll be in 0.92.1 :)
Thank you, su_v, Mc and Tav!

[su_v (suv-lp)]

JFTR - in latest release Inkscape 0.92.1 the baseline spacing of text in legacy files is adjusted on load. The behavior is enabled by default and can be disabled with a command line option (--no-convert-text-baseline-spacing) if needed.

Relevant commits in the stable release branch <lp:inkscape/0.92.x>:
http://bazaar.launchpad.net/~inkscape.dev/inkscape/0.92.x/revision/15338
http://bazaar.launchpad.net/~inkscape.dev/inkscape/0.92.x/revision/15339
http://bazaar.launchpad.net/~inkscape.dev/inkscape/0.92.x/revision/15350
http://bazaar.launchpad.net/~inkscape.dev/inkscape/0.92.x/revision/15351

The changes have been forward-ported to trunk <lp:inkscape> in
https://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/15481

Fixes for locale-related regression:
https://bazaar.launchpad.net/~inkscape.dev/inkscape/0.92.x/revision/15368
https://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/15503

[username132 (sean-van-der-smythe)]

Does this bug affect only files created prior to 0.92 or can it also affect files created with 0.92.1?