I am working on supporting non-inline CSS stylesheets, by using libcroco (a C library for CSS parsing and determining what CSS properties apply to a given element).
Not quite: libcroco 0.6 (the most recent released version) isn't really suitable for Inkscape's use for finding what properties apply to a given element: it requires a libxml in-memory representation of the document tree rather than allowing use of our preferred Inkscape::XML::Node. (See near end of http://www.inkscape.org/cgi-bin/wiki.pl?CSS_Support for more details.)
I have prepared some modifications to libcroco to make this more suitable for us, and I've gradually been feeding patches to the libcroco maintainer, but that will take some time for all the changes just to reach CVS, let alone be released as libcroco 0.7, let alone be available on a reasonable number of our users' systems.
There are two main ways of addressing this that I'm considering:
i) Put our own version of the rule selection code (cr-sel-eng.{c,h}) into Inkscape, and make it play nice with an installed (external) copy of libcroco 0.6.
ii) Put all of libcroco into the Inkscape source tree.
For (i), there are two aspects of "playing nicely with installed libcroco":
- Getting `#include "cr-foo.h"' to find cr-foo.h (usually referenced from user apps as `#include <libcroco/cr-foo.h>').
The simplest approach is to change all `#include "cr-foo.h"' to `#include <libcroco/cr-foo.h>'. This is my preferred approach at the moment.
It's tempting to try instead to get `-I/usr/include/libcroco-0.6/libcroco' (or whatever the path on the user's system) into our INCLUDES variable. This approach would be pleasing in its simplicity, and would also slightly reduce work in merging changes back into upstream libcroco. However, given that the exact path varies from system to system, the best I can think of involves adding to configure.ac something like
ink_cr_incl=`pkg-config --cflags libcroco-0.6 | sed 's, .*,/libcroco ,'` INCLUDES="$INCLUDES $ink_cr_incl"
(while retaining libcroco-0.6 in a PKG_CHECK_MODULES call, which would take care of LDFLAGS and error handling). Not too big a deal, but then neither is the #include-changing alternative.
- Avoiding linkage errors from having cr-sel-eng symbols both in Inkscape's internal libraries and also in installed libcroco. Is there some linker options to avoid this? Otherwise, the alternatives are either to #define all the external symbols of our own cr-sel-eng to something else, or to convert cr-sel-eng.c to C++, which I believe should give different linker symbols (mangled); we could wrap in a namespace{} too if we want.
My local copy uses the convert-to-C++ approach. It involved adding explicit casts in a dozen or so places (no implicit cast from void* in C++, no treating enums as ints).
Approach (ii) has at least the following advantages & disadvantages:
Advantages:
- All libcroco source files can stay in C and can retain their existing #include text.
- It guarantees us that libcroco is available. Otherwise we must choose between either:
- Requiring libcroco (and bundling on some platforms with poor package management -- can't they all just use Debian? ;-) ); or
- Using `#ifdef WITH_LIBCROCO' in the source code, and accepting that not all builds of 0.42 (or whatever) will be able to read SVG files using non-inline style.
Disadvantages:
- Makes Inkscape source tree about 1MB bigger, and increases Inkscape's memory usage at runtime.
- Any bug fixes to libcroco would have to wait until at least one release of Inkscape before being fixed. This is most of concern if the bug fix is security-related.
Licensing doesn't seem to be a problem: libcroco appears to be LGPL.
I haven't yet checked with the libcroco maintainer for his comments; I'll bcc him.
Can people add to this list of advantages & disadvantes, or suggest alternatives, or give other relevant information?
pjrm.
On Mon, 07 Mar 2005 13:42:58 +1100, Peter Moulder <Peter.Moulder@...38...> wrote:
Disadvantages:
- Makes Inkscape source tree about 1MB bigger, and increases Inkscape's memory usage at runtime.
Why is memory usage increased, compared to loading a lib?
- Any bug fixes to libcroco would have to wait until at least one release of Inkscape before being fixed. This is most of concern if the bug fix is security-related.
This means that the answer to your question (whether to include libcroco) depends on how frequent are the upstream libcroco updates. If it has stalled or is slow, we should better include it here. Otherwise, we should link. Including is easier for our users (less dependencies), and it has worked well for potrace (which hasn't stalled, but its updates are infrequent and, in part, driven by Inkscape activity). From what I know about libcroco, including it seems to be a better option.
bulia byak wrote:
On Mon, 07 Mar 2005 13:42:58 +1100, Peter Moulder <Peter.Moulder@...38...> wrote:
Disadvantages:
- Makes Inkscape source tree about 1MB bigger, and increases Inkscape's memory usage at runtime.
Why is memory usage increased, compared to loading a lib?
- Any bug fixes to libcroco would have to wait until at least one release of Inkscape before being fixed. This is most of concern if the bug fix is security-related.
This means that the answer to your question (whether to include libcroco) depends on how frequent are the upstream libcroco updates. If it has stalled or is slow, we should better include it here. Otherwise, we should link. Including is easier for our users (less dependencies), and it has worked well for potrace (which hasn't stalled, but its updates are infrequent and, in part, driven by Inkscape activity). From what I know about libcroco, including it seems to be a better option.
Speaking of which, I hope that nobody was affected by the quick update from Potrace1.6 ---> to --->1.7 that I performed during the weekend. It was a bugfix release from Peter.
Bob
bulia byak wrote:
This means that the answer to your question (whether to include libcroco) depends on how frequent are the upstream libcroco updates. If it has stalled or is slow, we should better include it here. Otherwise, we should link. Including is easier for our users (less dependencies), and it has worked well for potrace (which hasn't stalled, but its updates are infrequent and, in part, driven by Inkscape activity). From what I know about libcroco, including it seems to be a better option.
As another example, it looks like Mozilla is carrying their own copy of Cairo now. It is in the CVS tree at mozilla/gfx/cairo.
Bob
On Tue, 2005-03-08 at 12:16 -0600, Bob Jamison wrote:
As another example, it looks like Mozilla is carrying their own copy of Cairo now. It is in the CVS tree at mozilla/gfx/cairo.
Considering that Cairo is going to be going through a rather dramatic API shake-up sometime relatively soon, I hope that this is just a measure to keep Mozilla with SVG support building for now. I'd really hope that they take that out again later on. Especially for something which is supposed to be as generically useful as Cairo (lots of people in the X and GTK worlds are planning to make Cairo into the default display and print rendering system) it would be a shame if all apps waste memory by carrying around their own statically linked copies.
For something like potrace, which is unlikely to be in use by a whole lot of other stuff at the same time, and where Inkscape integration has grown to a significant part of the point of the library, including it and linking statically is a whole lot less bad.
Personally I think that the libcroco case sounds like it's somewhere in between. Librsvg depends on libcroco (as far as I can tell, at least that's what RPM tells me on my machine) so it's very likely that it's already installed for most users and thus not a big deal as a build dep. If you include it in the tree, please do make sure that it's possible to link dynamically to an external copy, if at all possible. Given that sort of random stuff is going to get tossed toward libcroco it can be considered somewhat security sensitive (re buffer overflows etc); that makes a shared librar convenient since it only needs a single update in order to fix a newly discovered security problem instead of fixes in all the apps that depend on it. Remember, by pulling something external into the tree, you're to some extent taking on responsibility for bugs (even security bugs!) in that code.
Cheers, Per
Quoting bulia byak <buliabyak@...400...>:
On Mon, 07 Mar 2005 13:42:58 +1100, Peter Moulder <Peter.Moulder@...38...> wrote:
Disadvantages:
- Makes Inkscape source tree about 1MB bigger, and increases Inkscape's memory usage at runtime.
Why is memory usage increased, compared to loading a lib?
It doesn't increase the memory usage so much as reduce the number of pages that can be shared between processes.
With static linking, every executable has its own copy of the library in memory as it runs (though multiple instances of the same executable share among themselves).
With dynamic linking, the library is not only shared on disk, but shared in memory too -- e.g. if three different executables are running that all link against the same version of libfoo, there will still only be one copy in memory that all three share.
Note that the sharing only applies to code, not data, and that code is demand-loaded, so you mostly only pay for the parts of the library you use.
-mental
On Tue, 8 Mar 2005 13:16:32 -0500, mental@...3... <mental@...3...> wrote:
Why is memory usage increased, compared to loading a lib?
It doesn't increase the memory usage so much as reduce the number of pages that can be shared between processes.
With static linking, every executable has its own copy of the library in memory as it runs (though multiple instances of the same executable share among themselves).
So, this argument is only applicable to libraries used by many different software. I guess this is not the case with libcroco, otherwise it would be widely available on distributions and we would not need to carry it. So, this seems to be another argument in favor of including it (at least until it becomes common enough).
On Tue, 2005-03-08 at 14:24 -0400, bulia byak wrote:
So, this argument is only applicable to libraries used by many different software. I guess this is not the case with libcroco, otherwise it would be widely available on distributions and we would not need to carry it. So, this seems to be another argument in favor of including it (at least until it becomes common enough).
How did you come to that conclusion? It appears to be a dependency for librsvg, so pretty much all Gnome users should have it installed at least. I think the reason Peter even considered importing the code is that some limitations in the current version need to be worked around if it is to be used.
Cheers, Per
On Tue, 08 Mar 2005 10:54:09 -0800, Per Bjornsson <perbj@...604...> wrote:
On Tue, 2005-03-08 at 14:24 -0400, bulia byak wrote:
So, this argument is only applicable to libraries used by many different software. I guess this is not the case with libcroco, otherwise it would be widely available on distributions and we would not need to carry it. So, this seems to be another argument in favor of including it (at least until it becomes common enough).
How did you come to that conclusion? It appears to be a dependency for librsvg, so pretty much all Gnome users should have it installed at least. I think the reason Peter even considered importing the code is that some limitations in the current version need to be worked around if it is to be used.
I didn't know about librsvg. From discussions with Peter, I got the impression that this library is not widely used and is currently stagnating or almost stagnating, hence my suggestion to import it.
On Tue, 2005-03-08 at 14:58 -0400, bulia byak wrote:
I didn't know about librsvg. From discussions with Peter, I got the impression that this library is not widely used and is currently stagnating or almost stagnating, hence my suggestion to import it.
OK, perhaps I need to moderate my statement a bit, after a little nudge in a private e-mail: using libcroco for CSS parsing is the default for librsvg, but librsvg can be built without libcroco. Fedora does build against it, I don't have other distros available here to check.
I didn't realize that libcroco was so slow-moving; I'd still think that it would make sense to make changes upstream if possible, but that does make it seem more attractive to keep a private copy for the time being. However, in the long run having it separate is nice; there might be other applications too that want CSS handling capabilities.
Cheers, Per
Quoting Per Bjornsson <perbj@...604...>:
How did you come to that conclusion? It appears to be a dependency for librsvg, so pretty much all Gnome users should have it installed at least.
Whoops, you're right..
I think the reason Peter even considered importing the code is that some limitations in the current version need to be worked around if it is to be used.
Basically stock libcroco requires libxml's XML tree representation, which we don't use. IIRC peter is abstracting the tree representation for us.
It was actually originally something I had promised Dodji (the libcroco maintainer) I would do a year or two ago, but I never got around to it. :/
-mental
Quoting bulia byak <buliabyak@...400...>:
So, this argument is only applicable to libraries used by many different software. I guess this is not the case with libcroco, otherwise it would be widely available on distributions and we would not need to carry it. So, this seems to be another argument in favor of including it (at least until it becomes common enough).
Correct.
On Tue, Mar 08, 2005 at 02:24:07PM -0400, bulia byak wrote:
On Tue, 8 Mar 2005 13:16:32 -0500, mental@...3... <mental@...3...> wrote:
Why is memory usage increased, compared to loading a lib?
It doesn't increase the memory usage so much as reduce the number of pages that can be shared between processes.
Just to be explicit: this means that using our own copy of libcroco (rather than linking against the system-installed one) increases memory usage when and only when there are other processes running that are linked against the system libcroco.
librsvg can optionally be built linked against libcroco. At least Fedora and Debian do so; I'd guess that most distributions do.
nautilus in turn links against librsvg, which I think means that ~all gnome users will already have libcroco in memory: I think nautilus provides the desktop icons.
To see if libcroco is in memory right now on your system, try ldd /proc/[1-9]*/exe 2>/dev/null | grep croco (Do it as root if you have gdm or if there are X sessions running under more than one user.)
pjrm.
On Mon, Mar 07, 2005 at 01:42:58PM +1100, Peter Moulder wrote:
There are two main ways of addressing this that I'm considering:
i) Put our own version of the rule selection code (cr-sel-eng.{c,h}) into Inkscape, and make it play nice with an installed (external) copy of libcroco 0.6.
ii) Put all of libcroco into the Inkscape source tree.
I am generally against putting external libraries into source trees. They have a tendency to bitrot very very badly. If it can be done in such a way that the separation between inkscape and the lib is clear, and any changes made to it are sent to the upstream, then I guess it's okay. I still think it's dangerous. :)
Quoting Kees Cook <inkscape@...62...>:
I am generally against putting external libraries into source trees. They have a tendency to bitrot very very badly. If it can be done in such a way that the separation between inkscape and the lib is clear, and any changes made to it are sent to the upstream, then I guess it's okay. I still think it's dangerous. :)
Well, a problem is that its maintainer is AWOL, as far as I can tell (no activity on the libcroco list for a LONG, long time...). Nobody to push patches to.
-mental
On Tue, Mar 08, 2005 at 02:30:00PM -0500, mental@...3... wrote:
Well, a problem is that its maintainer is AWOL, as far as I can tell (no activity on the libcroco list for a LONG, long time...). Nobody to push patches to.
Then we should produce a inkscape-version of it (rather than importing) so that everyone can benefit from our improvements. :)
On Tue, Mar 08, 2005 at 02:30:00PM -0500, mental@...3... wrote:
Quoting Kees Cook <inkscape@...62...>:
[it is desirable that] any changes made to it [be] sent to the upstream
Well, a problem is that its maintainer is AWOL, as far as I can tell (no activity on the libcroco list for a LONG, long time...). Nobody to push patches to.
Dodji was absent for a while, but has been back for about a month. He has already applied a few of the patches I've sent, though progress is a little slow at the moment.
pjrm.
On Tue, 2005-03-08 at 20:11, Peter Moulder wrote:
On Tue, Mar 08, 2005 at 02:30:00PM -0500, mental@...3... wrote:
Quoting Kees Cook <inkscape@...62...>:
[it is desirable that] any changes made to it [be] sent to the upstream
Well, a problem is that its maintainer is AWOL, as far as I can tell (no activity on the libcroco list for a LONG, long time...). Nobody to push patches to.
Dodji was absent for a while, but has been back for about a month. He has already applied a few of the patches I've sent, though progress is a little slow at the moment.
That sounds reassuring then.
-mental
participants (7)
-
unknown@example.com
-
Bob Jamison
-
bulia byak
-
Kees Cook
-
MenTaLguY
-
Per Bjornsson
-
Peter Moulder