Inkscape

Update: color managed view should now work.

Regards, Krzysztof

On Apr 7, 2011, at 7:43 PM, Krzysztof Kosiński wrote:

> Update: color managed view should now work.


Rock!!!

On Fri, 2011-04-08 at 01:59 +0200, Krzysztof Kosiński wrote:
> Hello
> 
> I have updated my Cairo branch today, as a step towards merging it.

Excellent!

> There is one new regression, namely feImage does not handle aspect
> ratio. It will be fixed before merging.

I can have a look at this next week if you haven't already fixed it.

> IIRC, there is a problem with CMS output, the editing controls are not
> using custom XOR blending, and there are build failures on Windows.
> Does anyone remember any other bugs?

I have seen a problem with clipping not working reliably (object
sometimes just partially clipped) but I haven't had time to investigate.
 

					Tav

2011/4/7 Krzysztof Kosiński <tweenk.pl@...>

> Hello
>
> I have updated my Cairo branch today, as a step towards merging it.
> There is one new regression, namely feImage does not handle aspect
> ratio. It will be fixed before merging.
> IIRC, there is a problem with CMS output, the editing controls are not
> using custom XOR blending, and there are build failures on Windows.
> Does anyone remember any other bugs?
>

Was building on OSX resolved as well? If not, that's still an outstanding
bug too.

Cheers,
Josh

On 8/4/11 19:40, Josh Andler wrote:
> 2011/4/7 Krzysztof Kosiński <tweenk.pl@...>
> 
>> I have updated my Cairo branch today, as a step towards merging it.
>> There is one new regression, namely feImage does not handle aspect
>> ratio. It will be fixed before merging.
>> IIRC, there is a problem with CMS output, the editing controls are not
>> using custom XOR blending, and there are build failures on Windows.
>> Does anyone remember any other bugs?
> 
> Was building on OSX resolved as well? If not, that's still an outstanding
> bug too.

Building r9582 of the gsoc-cairo branch on osx fails with the same error
message as before.
Has anyone tested on Windows?


~suv

On 8-4-2011 1:59, Krzysztof Kosiński wrote:
> Hello
>
> I have updated my Cairo branch today, as a step towards merging it.
> There is one new regression, namely feImage does not handle aspect
> ratio. It will be fixed before merging.
> IIRC, there is a problem with CMS output, the editing controls are not
> using custom XOR blending, and there are build failures on Windows.

Can you give specific people (like me :P) commit access, so I can help 
with the build problems?

Thanks,
Johan

2011/4/8 Johan Engelen <jbc.engelen@...>:
> Can you give specific people (like me :P) commit access, so I can help
> with the build problems?

Looks like there is now way in Launchpad to do this. I'm now uploading
it with ~inkscape.dev as the owner.

Regards, Krzysztof

On 8-4-2011 22:30, Krzysztof Kosiński wrote:
> 2011/4/8 Johan Engelen<jbc.engelen@...>:
>> Can you give specific people (like me :P) commit access, so I can help
>> with the build problems?
> Looks like there is now way in Launchpad to do this.

I'm not surprised...

> I'm now uploading
> it with ~inkscape.dev as the owner.

Thanks. I've (temporarily) fixed the Windows build.
I get a crash on startup and have to work around an assert.
src/libnrtype/FontInstance.cpp, line 560:
                                 //g_assert(polyCurve->cpfx % 2 == 0);
                                 if (polyCurve->cpfx % 2 != 0) return;

One thing I noticed is funny: the diamond-shaped knots are not symmetric 
any more. The topleft edge is shorter than the others :)
Not sure whether I like the "better looking" knots now. They look more 
part of the figure instead of part of the UI (the old ones are more 
pixelated, and 1 pixel stroked).

Ciao,
   Johan

W dniu 8 kwietnia 2011 22:37 użytkownik Johan Engelen
<jbc.engelen@...> napisał:
> One thing I noticed is funny: the diamond-shaped knots are not symmetric any
> more. The topleft edge is shorter than the others :)
> Not sure whether I like the "better looking" knots now. They look more part
> of the figure instead of part of the UI (the old ones are more pixelated,
> and 1 pixel stroked).

Yes, the knots need fixing - the old display was better. It requires
some pixel manipulation wizardry that cannot be duplicated using Cairo
compositing operators, but I have this almost figured out.

The problem in FontInstance probably results from me gutting out
raster_font completely - we are rendering the fonts as vector curves
now. Even better would be to directly render the Pango layouts.

Regards, Krzysztof

W dniu 8 kwietnia 2011 22:30 użytkownik Krzysztof Kosiński
<tweenk.pl@...> napisał:
> Looks like there is no way in Launchpad to do this. I'm now uploading
> it with ~inkscape.dev as the owner.

The inkscape.dev-owned branch is now available at:
lp:~inkscape.dev/inkscape/cairo-rendering

Regards, Krzysztof

On 8-4-2011 22:45, Krzysztof Kosiński wrote:
> W dniu 8 kwietnia 2011 22:30 użytkownik Krzysztof Kosiński
> <tweenk.pl@...>  napisał:
>> Looks like there is no way in Launchpad to do this. I'm now uploading
>> it with ~inkscape.dev as the owner.
> The inkscape.dev-owned branch is now available at:
> lp:~inkscape.dev/inkscape/cairo-rendering

Cool. I have committed my build fixing changes.
One final required thing I did not commit: I disabled some code in 
win32.cpp to get a complete build. But it really needs to be fixed, 
instead of disabled.
Can you look into this Krzysztof?

/src/extension/internal/win32.cpp
line 316
line 334
etc, use pixblocks, and should be converted to cairo stuff.

Thanks!
   Johan

W dniu 8 kwietnia 2011 23:46 użytkownik Johan Engelen
<jbc.engelen@...> napisał:
> Cool. I have committed my build fixing changes.
> One final required thing I did not commit: I disabled some code in win32.cpp
> to get a complete build. But it really needs to be fixed, instead of
> disabled.
> Can you look into this Krzysztof?
>
> /src/extension/internal/win32.cpp
> line 316
> line 334
> etc, use pixblocks, and should be converted to cairo stuff.

This has something to do with printing. I thought we were using Cairo
and GtkPrint for this. Does anyone know how printing is done on
Windows?

Regards, Krzysztof

On 8/4/11 22:45, Krzysztof Kosiński wrote:
> W dniu 8 kwietnia 2011 22:30 użytkownik Krzysztof Kosiński
> <tweenk.pl@...> napisał:
>> Looks like there is no way in Launchpad to do this. I'm now uploading
>> it with ~inkscape.dev as the owner.
> 
> The inkscape.dev-owned branch is now available at:
> lp:~inkscape.dev/inkscape/cairo-rendering

Building the branch 'cairo-renderer' (r9586) on Mac OS X 10.5.8 this
time ran through without failure, but inkscape crashes when launching
the GUI ('inkscape --version' or 'inkscape--verb-list' is ok).
Preferences are default (new).

Backtrace attached - a proposed fix by Johan on jabber didn't help:

> in /display/nr-style.cpp
> try changing from line 198 to :
> void NRStyle::update()
> {
>     // force pattern update
>     if (fill_pattern) cairo_pattern_destroy(fill_pattern);
>     if (stroke_pattern) cairo_pattern_destroy(stroke_pattern);
>     fill_pattern = NULL;
>     stroke_pattern = NULL;
> }
> maybe that will fix your crash

Dependencies (using the X11 backend of GTK+):
gtk2                           @2.24.4
gtkmm                          @2.22.0
glib2                          @2.28.5
glibmm                         @2.24.2
cairo                          @1.10.2


~suv

W dniu 9 kwietnia 2011 01:56 użytkownik ~suv
<suv-sf@...> napisał:
> #0  0x02fbadfc in cairo_pattern_destroy ()
> #1  0x0027edc7 in NRStyle::update () at display/nr-style.cpp:201
> #2  0x0027edc7 in NRStyle::set (this=0xf0f0f0f0, style=0x6887b70) at display/nr-style.cpp:140

0xf0f0f0f0 for "this" is obviously wrong, the stack is getting
corrupted for some reason.

> #3  0x00258941 in nr_arena_shape_set_style (shape=0x643ce00, style=0x6887b70) at display/nr-arena-shape.cpp:543

The "style" pointer looks correct here, so something went awry in
nr_arena_shape_set_style. At the moment I have no idea what is going
on.

Can you try compiling without optimization (-O0), or adding
"g_return_if_fail(style != NULL);" at the top of
nr_arena_shape_set_style()?

Regards, Krzysztof

On 9/4/11 02:12, Krzysztof Kosiński wrote:
> W dniu 9 kwietnia 2011 01:56 użytkownik ~suv
> <suv-sf@...> napisał:
>> #0  0x02fbadfc in cairo_pattern_destroy ()
>> #1  0x0027edc7 in NRStyle::update () at display/nr-style.cpp:201
>> #2  0x0027edc7 in NRStyle::set (this=0xf0f0f0f0, style=0x6887b70) at display/nr-style.cpp:140
> 
> 0xf0f0f0f0 for "this" is obviously wrong, the stack is getting
> corrupted for some reason.
> 
>> #3  0x00258941 in nr_arena_shape_set_style (shape=0x643ce00, style=0x6887b70) at display/nr-arena-shape.cpp:543
> 
> The "style" pointer looks correct here, so something went awry in
> nr_arena_shape_set_style. At the moment I have no idea what is going
> on.
> 
> Can you try compiling without optimization (-O0), or adding
> "g_return_if_fail(style != NULL);" at the top of
> nr_arena_shape_set_style()?

Editing 'src/display/nr-arena-shape.cpp' didn't help:

LeWitt:inkscape-cairo2 suv$ mvim src/display/nr-arena-shape.cpp
LeWitt:inkscape-cairo2 suv$ bzr diff !$
bzr diff src/display/nr-arena-shape.cpp
=== modified file 'src/display/nr-arena-shape.cpp'
--- src/display/nr-arena-shape.cpp      2011-04-07 23:42:04 +0000

+++ src/display/nr-arena-shape.cpp	2011-04-09 00:15:56 +0000
@@ -533,6 +533,7 @@
 void
 nr_arena_shape_set_style(NRArenaShape *shape, SPStyle *style)
 {
+    g_return_if_fail(style != NULL);
     g_return_if_fail(shape != NULL);
     g_return_if_fail(NR_IS_ARENA_SHAPE(shape));


LeWitt:inkscape-cairo2 suv$


Backtrace attached. I'll do a full rebuild changing optimization from
'-O3' to '-O0'.


~suv

On 9/4/11 02:12, Krzysztof Kosiński wrote:
> W dniu 9 kwietnia 2011 01:56 użytkownik ~suv
> <suv-sf@...> napisał:
>> #0  0x02fbadfc in cairo_pattern_destroy ()
>> #1  0x0027edc7 in NRStyle::update () at display/nr-style.cpp:201
>> #2  0x0027edc7 in NRStyle::set (this=0xf0f0f0f0, style=0x6887b70) at display/nr-style.cpp:140
> 
> 0xf0f0f0f0 for "this" is obviously wrong, the stack is getting
> corrupted for some reason.
> 
>> #3  0x00258941 in nr_arena_shape_set_style (shape=0x643ce00, style=0x6887b70) at display/nr-arena-shape.cpp:543
> 
> The "style" pointer looks correct here, so something went awry in
> nr_arena_shape_set_style. At the moment I have no idea what is going
> on.
> 
> Can you try compiling without optimization (-O0), or adding
> "g_return_if_fail(style != NULL);" at the top of
> nr_arena_shape_set_style()?

Same error after 'make clean' and rebuilding with '-O0'.


~suv

W dniu 9 kwietnia 2011 03:30 użytkownik ~suv
<suv-sf@...> napisał:
> Same error after 'make clean' and rebuilding with '-O0'.
>
>
> ~suv
>
> Program received signal EXC_BAD_ACCESS, Could not access memory.
> Reason: KERN_INVALID_ADDRESS at address: 0xf0f0f0f4
> 0x03c77dfc in cairo_pattern_destroy ()
> (gdb) bt
> #0  0x03c77dfc in cairo_pattern_destroy ()
> #1  0x002e5e94 in NRStyle::update (this=0x713eec4) at display/nr-style.cpp:201
> #2  0x002e6974 in NRStyle::set (this=0x713eec4, style=0x7587aa0) at display/nr-style.cpp:140

This looks like uninitialized memory in NRStyle. I added explicit NULL
initializers in rev. 9588, see whether it fixes the crash.

Regards, Krzysztof

On 9/4/11 03:48, Krzysztof Kosiński wrote:
> W dniu 9 kwietnia 2011 03:30 użytkownik ~suv
> <suv-sf@...> napisał:
>> Same error after 'make clean' and rebuilding with '-O0'.
>>
>>
>> ~suv
>>
>> Program received signal EXC_BAD_ACCESS, Could not access memory.
>> Reason: KERN_INVALID_ADDRESS at address: 0xf0f0f0f4
>> 0x03c77dfc in cairo_pattern_destroy ()
>> (gdb) bt
>> #0  0x03c77dfc in cairo_pattern_destroy ()
>> #1  0x002e5e94 in NRStyle::update (this=0x713eec4) at display/nr-style.cpp:201
>> #2  0x002e6974 in NRStyle::set (this=0x713eec4, style=0x7587aa0) at display/nr-style.cpp:140
> 
> This looks like uninitialized memory in NRStyle. I added explicit NULL
> initializers in rev. 9588, see whether it fixes the crash.

Success! :)

Inkscape (cairo-renderer r9588) up and running on osx too (this is still
with '-O0'). I'll do some tests and rebuild with '-O3' tomorrow, and
maybe also a quartz-based version.

Thank you for making this work,
~suv

On Apr 8, 2011, at 6:48 PM, Krzysztof Kosiński wrote:

> This looks like uninitialized memory in NRStyle. I added explicit NULL
> initializers in rev. 9588, see whether it fixes the crash.


Well, since we're C++ we should be using 0 instead of NULL.  :-)

But more importantly, this is exactly the sort of issue that Valgrind is very good at spotting and pointing out. 

When problems like this (subtle corruptions or crashing) are hit, running under valgrind should be a first step. It will generally catch a problem when it first occurs, not later on after it has cascaded enough to actually cause a crash, etc.


If anyone needs more detail on the wiki for this, let me know. Also drop on in to IRC to hash it out (that really helps).

On 2011-04-09 0:12, Krzysztof Kosiński wrote:
> W dniu 8 kwietnia 2011 23:46 użytkownik Johan Engelen
> <jbc.engelen@...> napisał:
>> Cool. I have committed my build fixing changes.
>> One final required thing I did not commit: I disabled some code in win32.cpp
>> to get a complete build. But it really needs to be fixed, instead of
>> disabled.
>> Can you look into this Krzysztof?
>>
>> /src/extension/internal/win32.cpp
>> line 316
>> line 334
>> etc, use pixblocks, and should be converted to cairo stuff.
> This has something to do with printing. I thought we were using Cairo
> and GtkPrint for this. Does anyone know how printing is done on
> Windows?
I just had a look (on Windows of course) and as far I can tell this code
is unused. I can cut it out (as well as all relevant references and id's
that I've been able to find) and I can still print. Apparently it used
to have some function, and we even had a print preview and such, but I
think at some point this was superseded by a new system and the old
stuff was just left. In file.h there is even a comment that some
print(preview) related functions are probably redundant, which
apparently dates back to SVN(!!!) revision 1600-something.

In short: I'd like to try and cut out some of this apparently unused
code, cleaning up our code base and "fixing" an issue with the cairo
branch at the same time. But, it seems a little scary to just remove a
whole lot of code that apparently at least used to have some function,
so if anyone has a reason to keep this code around, please let me know.

On Sat, Apr 9, 2011 at 4:13 AM, Jasper van de Gronde <
th.v.d.gronde@...> wrote:

> On 2011-04-09 0:12, Krzysztof Kosiński wrote:
> > W dniu 8 kwietnia 2011 23:46 użytkownik Johan Engelen
> > <jbc.engelen@...> napisał:
> >> Cool. I have committed my build fixing changes.
> >> One final required thing I did not commit: I disabled some code in
> win32.cpp
> >> to get a complete build. But it really needs to be fixed, instead of
> >> disabled.
> >> Can you look into this Krzysztof?
> >>
> >> /src/extension/internal/win32.cpp
> >> line 316
> >> line 334
> >> etc, use pixblocks, and should be converted to cairo stuff.
> > This has something to do with printing. I thought we were using Cairo
> > and GtkPrint for this. Does anyone know how printing is done on
> > Windows?
> I just had a look (on Windows of course) and as far I can tell this code
> is unused. I can cut it out (as well as all relevant references and id's
> that I've been able to find) and I can still print. Apparently it used
> to have some function, and we even had a print preview and such, but I
> think at some point this was superseded by a new system and the old
> stuff was just left. In file.h there is even a comment that some
> print(preview) related functions are probably redundant, which
> apparently dates back to SVN(!!!) revision 1600-something.
>
> In short: I'd like to try and cut out some of this apparently unused
> code, cleaning up our code base and "fixing" an issue with the cairo
> branch at the same time. But, it seems a little scary to just remove a
> whole lot of code that apparently at least used to have some function,
> so if anyone has a reason to keep this code around, please let me know.
>

Was the code not commented out? If not... *ugh*. I would say that you should
kill it all, test, and if it still works it's a win for spring cleaning!

Cheers,
Josh

On Fri, Apr 8, 2011 at 11:13 PM, Jon Cruz <jon@...> wrote:

>
> On Apr 8, 2011, at 6:48 PM, Krzysztof Kosiński wrote:
>
> > This looks like uninitialized memory in NRStyle. I added explicit NULL
> > initializers in rev. 9588, see whether it fixes the crash.
>
>
> Well, since we're C++ we should be using 0 instead of NULL.  :-)
>
> But more importantly, this is exactly the sort of issue that Valgrind is
> very good at spotting and pointing out.
>
> When problems like this (subtle corruptions or crashing) are hit, running
> under valgrind should be a first step. It will generally catch a problem
> when it first occurs, not later on after it has cascaded enough to actually
> cause a crash, etc.
>
> If anyone needs more detail on the wiki for this, let me know. Also drop on
> in to IRC to hash it out (that really helps).
>

Krzysztof,

It appears that the new renderer doesn't release memory on zooming
operations. Without any modifications to the document, just zooming in and
out a number of times shows memory growth and memory that isn't released. If
it's known and expected, okay, but it's just an observation I figured I
would point out. The document I sent you off-list is a good test file for
this behavior btw. (a follow-up to a valgrind reply seemed like it was a
relevant place to comment)

Cheers,
Josh

On 10/4/11 00:29, Josh Andler wrote:

> It appears that the new renderer doesn't release memory on zooming
> operations. Without any modifications to the document, just zooming in and
> out a number of times shows memory growth and memory that isn't released. If
> it's known and expected, okay, but it's just an observation I figured I
> would point out. The document I sent you off-list is a good test file for
> this behavior btw. (a follow-up to a valgrind reply seemed like it was a
> relevant place to comment)

Does you sample file contain (fairly complex) patterns?

We do have two reports about zooming-related memory leaks with files
containing patterns for the current renderer already:

706294 Huge memory leak when zooming in & out complex pattern
       <https://bugs.launchpad.net/inkscape/+bug/706294>

608944 Enormous memory consumption (memory leak?
       <https://bugs.launchpad.net/inkscape/+bug/608944>


~suv

2011/4/9 ~suv <suv-sf@...>

> On 10/4/11 00:29, Josh Andler wrote:
>
> > It appears that the new renderer doesn't release memory on zooming
> > operations. Without any modifications to the document, just zooming in
> and
> > out a number of times shows memory growth and memory that isn't released.
> If
> > it's known and expected, okay, but it's just an observation I figured I
> > would point out. The document I sent you off-list is a good test file for
> > this behavior btw. (a follow-up to a valgrind reply seemed like it was a
> > relevant place to comment)
>
> Does you sample file contain (fairly complex) patterns?


Nope. Paths, LPEs, and filters.

Cheers,
Josh

W dniu 9 kwietnia 2011 08:13 użytkownik Jon Cruz <jon@...> napisał:
> When problems like this (subtle corruptions or crashing) are hit, running under valgrind should be a first step. It will generally catch a problem when it first occurs, not later on after it has cascaded enough to actually cause a crash, etc.

Last time I tried using valgrind, Inkscape crashed hard, and it was
apparently due to our usage of libgc. Did anyone manage to run
Inkscape under valgrind?

Regards, Krzysztof

2011/4/9 Jasper van de Gronde <th.v.d.gronde@...>:
> In short: I'd like to try and cut out some of this apparently unused
> code, cleaning up our code base and "fixing" an issue with the cairo
> branch at the same time. But, it seems a little scary to just remove a
> whole lot of code that apparently at least used to have some function,
> so if anyone has a reason to keep this code around, please let me know.

Kill it with fire.

The code was really bad in the first place, because it unconditionally
rasterized the image before printing.

Regards, Krzysztof

I had a while ago, John Bintz has too.

Cheers,
Josh
On Apr 9, 2011 7:00 PM, "Krzysztof Kosiński" <tweenk.pl@...> wrote:
> W dniu 9 kwietnia 2011 08:13 użytkownik Jon Cruz <jon@...>
napisał:
>> When problems like this (subtle corruptions or crashing) are hit, running
under valgrind should be a first step. It will generally catch a problem
when it first occurs, not later on after it has cascaded enough to actually
cause a crash, etc.
>
> Last time I tried using valgrind, Inkscape crashed hard, and it was
> apparently due to our usage of libgc. Did anyone manage to run
> Inkscape under valgrind?
>
> Regards, Krzysztof
>
>
------------------------------------------------------------------------------
> Xperia(TM) PLAY
> It's a major breakthrough. An authentic gaming
> smartphone on the nation's most reliable network.
> And it wants your games.
> http://p.sf.net/sfu/verizon-sfdev
> _______________________________________________
> Inkscape-devel mailing list
> Inkscape-devel@...
> https://lists.sourceforge.net/lists/listinfo/inkscape-devel

On 8/4/11 01:59, Krzysztof Kosiński wrote:

> I have updated my Cairo branch today, as a step towards merging it.
> There is one new regression, namely feImage does not handle aspect
> ratio. It will be fixed before merging.
> IIRC, there is a problem with CMS output, the editing controls are not
> using custom XOR blending, and there are build failures on Windows.
> Does anyone remember any other bugs?

a) outline view mode

cairo-renderer r9589, OS X 10.5.8 (i386), Apple's GCC 4.2.1, no OpenMP:
- cairo 1.10.2, GTK+/X11
- cairo 1.11.2, GTK+/Quartz

Outline view mode uses white to draw the outlines: they are not visible
on default backgrounds (white). The color of the outline apparently is
not inverted (or xor-composited (?)) when using a dark background, either.

Currently, outline mode is only usable with a solid, darker background.


b) multi-threading / OpenMP

As described earlier [1], OpenMP is not detected by the configure script
when building with Apple's GCC 4.2.1 (although the man page states
otherwise, and e.g. 'libgomp.a' and the omp header do exist).

Testing a build of the cairo-renderer branch using plain GCC 4.2.4
installed via MacPorts does enable OpenMP (-fopenmp), but when used in
rendering, Inkscape randomly hangs (apparently waiting for something to
finish(?) - see attached backtrace when interrupting Inkscape after it
no longer accepts any input).

The build with GCC 4.2.4 also produced several warnings from the linker
("ld warning: can't find atom for N_GSYM stabs (...)").

I will try with newer GCC versions [2], but so far my earlier tests
building inkscape trunk with GCC 4.5 and GCC 4.4 had other serious
issues (crashes, or e.g. broken text rendering on canvas).

IMHO with the new renderer, we should get OpenMP working on OS X as
well. Any input or help - maybe it just needs adjusted compiler and
linker flags (?) - would be appreciated.


~suv


[1] <http://article.gmane.org/gmane.comp.graphics.inkscape.devel/34491>
[2] currently installed GCC versions (XCode 3.1.4, MacPorts):
     gcc-4.0      (4.0.1, Apple)
     gcc-4.2      (4.2.1, Apple)
     gcc-mp-4.2   (4.2.4, MacPorts)
     gcc-mp-4.3   (4.3.5, MacPorts)
     gcc-mp-4.4   (4.4.5, Macports)
     gcc-mp-4.5   (4.5.2, MacPorts)