Code review comment for lp:~valavanisalex/inkscape/fix-1156664

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

On 2015-11-13 24:49 (+0100), Alex Valavanis wrote:
> I've tested on Ubuntu 15.10 with and without Potrace, and using
> either Autotools or CMake.
>
> I'd appreciate someone checking:
> (a) does this work on Windows and OS X?

Tested successfully with trunk r14478 (GTK2) on OS X 10.7.5 with and
without Potrace, for Autotools- as well as CMake-based build system. Not
tested specifically with osx packaging (it should not cause issues there
though - the external linked library will be copied automagically into
the app bundle, and the library paths rewritten).

Uninformed question wrt potrace: I noticed that in MacPorts (package
manager for OS X) potrace's configure options for metric units
(--enable-metric) and default page size (--enable-a4) are offered as
variants - are these only relevant for the command line tools installed
by potrace, or would different defaults somehow affect trace mode /
flood fill in Inkscape?

> (b) is my CMake implementation sensible?

I noticed that cmake does not inform the user if potrace is not found
(it does report it if found though). Maybe let users know if features
are disabled due to missing optional dependencies?

(c) Independent of build system: it might be safer to disable the
related verbs too (tool, dialog), if compiling without potrace: inkscape
e.g. crashes if launched with one of the (defunct) verbs on the command
line:

$ inkscape --verb=ToolPaintBucket

or when switching tools using keyboard shortcuts (Shift+F7).

« Back to merge proposal