2011/10/8 Krzysztof Kosiński :

Just a quick note: I've been ill for the past week, which messed up some of my plans.

Currently the visual bbox calculation is done in a very inefficient way using livarot. It could potentially be improved by using cairo_stroke_extents instead.

Oh? I thought we ditched it entirely. Where else do we still use it?

Alexandre Prokoudine http://libregraphicsworld.org

Krzysztof Kosiński wrote:

Currently the visual bbox calculation is done in a very inefficient way using livarot. It could potentially be improved by using cairo_stroke_extents instead.

Thanks for all the replies/explanations. Ok, I'll positively wait for an improvement on this side. But maybe it's worth saying that I always use geometrical bounding boxes, so I can't understand why all these useless calculations on strokes are performed anyway (if I correctly understand). Also, I've never experienced such problems so they must have been introduced by some recent modification: could you point it out so, for now, I can revert to just before it? Regards. Luca

~suv-2 wrote:

The initial message of the thread I linked to earlier gives the revision number when libnr was removed:

Ok, thanks. I hoped not to have to go so much backward but indeed rev 10588 works as expected. Unfortunately, I'll have some difficulties in checking and appreciating Diederik's progresses (I've read "3) groundwork for tangential/perpendicular snapping" in rev 10672 comment :)

Krzysztof Kosiński wrote:

The problem is a bit deeper. There was a lot of code that used the "approximate" bounding box, which is not well defined, but roughly corresponds to the visual bounding box and is easy to calculate. I changed most code that used approximate bbox (which was the default) to use the visual bbox and removed the default value from methods that take a bounding box type, to force people to specify which kind of box they mean. As I worked through the code it became apparent that a lot of it is wrong and should use the geometric box, but this could cause regressions.

Does this mean that there's still a lot of work left before seeing a correct management of visual/geometric bounding box? If so, why not still keeping the quick "approximate" bounding box (at least for now) that worked so well instead of forcing the use of the much heavier visual one in all those cases where it's not really needed? Implementations would still remain wrong until they are corrected to use the geometric one. If I correctly understood the problem, of course.

What I surely didn't understand is why the overload rises with the number of objects present in the drawing: if I have a lot of rectangles, moving a single one is a pain while it's not when I have only a couple. I can't catch the relationship with the complexity of bounding box calculation (and, anyway, we are speaking of rectangles...).

Regards. Luca

On Nov 9, 2011, at 3:22 AM, Krzysztof Kosiński wrote:

The calculation is done too many times. It should only be done for the rectangle which is moved. I think somewhere there is an uncached call that should use the geometric bounding box, but uses the visual one instead.

Thanks for the info.

Do you think it might be good to start a bit of a page on the wiki going to keep short notes on these?

We might be able to get some support for helping you hunt these down if we can get some help going.

Thanks.

On 2011-12-29 0:38, Alvin Penner wrote:

 On my machine, with this svg file, it takes 5 seconds to calculate the

bounding box. The time required depends on the stroke. If I change the stroke from 0.5 px to 10 px, it takes only 1 second, but changing back from 10 px to 0.5 px takes 8 seconds, using the Fill and Stroke editor to modify stroke. ... It appears that we need a compromise of sorts: if the path touches the (geometric) bbox tangentially, then we can ignore endcap style and use the half stroke width as in APPROXIMATE_BBOX (which is very fast), and if the path endpoint is touching the (geometric) bbox then we need to compensate for endcap style as is currently being done.

This is tricky, as we do not only need to take endpoints into account, but also corners of paths. I am a little surprised by your report that apparently calculating the bounding box takes an amount of time that depends on the stroke width. Or did I misunderstand your report?

The purpose of the Path::Coalesce(double tresh) routine appears to be to detect whether successive segments on a multi-segmented path are smooth continuations of each other, and to merge them in that case. To do this a numerical tolerance is used to determine what is 'smooth', and the tolerance is based on the stroke width in this particular case. One can increase the likelihood of detecting a smooth join by increasing the stroke width, which is why the execution time depends (quite strongly) on stroke width. However, when calculating a bbox, there does not appear to be any practical reason for using this routine. The bbox will clearly not be affected by the merge of two segments if they are smooth, and the only obvious benefit is faster execution speed, which in this case has not been achieved. If I comment out the call to Path::Coalesce in line 784 of splivarot.cpp, then the execution speed returns back to a value that is comparable to Inkscape 0.48.2 as far as I can tell, while the bbox is unaffected by this change. The visual bbox obtained in this way contains more information than the bbox in Inkscape 0.48.2, since it responds to changes in endcap style and join style. Any objections if I comment out this line of code?

On 2-1-2012 1:00, Alvin Penner wrote:

 The purpose of the Path::Coalesce(double tresh) routine appears to be to

detect whether successive segments on a multi-segmented path are smooth continuations of each other, and to merge them in that case. To do this a numerical tolerance is used to determine what is 'smooth', and the tolerance is based on the stroke width in this particular case. One can increase the likelihood of detecting a smooth join by increasing the stroke width, which is why the execution time depends (quite strongly) on stroke width. However, when calculating a bbox, there does not appear to be any practical reason for using this routine. The bbox will clearly not be affected by the merge of two segments if they are smooth, and the only obvious benefit is faster execution speed, which in this case has not been achieved. If I comment out the call to Path::Coalesce in line 784 of splivarot.cpp, then the execution speed returns back to a value that is comparable to Inkscape 0.48.2 as far as I can tell, while the bbox is unaffected by this change. The visual bbox obtained in this way contains more information than the bbox in Inkscape 0.48.2, since it responds to changes in endcap style and join style. Any objections if I comment out this line of code?

Yes.

The item_outline function is not only used for bbox calculations. It is also used when converting 'object to path'.

Please make a 'separate' path outline function for the bbox calculation. Could be as simple as Geom::PathVector* item_outline(SPItem const *item, bool coalesce = false)

Thanks a bunch for looking into the performance issues, Johan

On Jan 2, 2012, at 3:08 PM, Krzysztof Kosiński wrote:

Awesome analysis! Thanks a lot :)

As Johan said, it's better to add a parameter that controls the use of Coalesce, since this function is also used in the stroke-to-path functionality.

Sometimes it's good to have an optional parameter, but then again sometimes it helps to have a separate function with an explicitly different name (even if at the implementation they end up calling a shared common function with a parameter).

Personally I think it really comes down to which one will make the code that uses these easier to read. And I do dislike the name "item_outline()" as it seems somewhat vague. A name more like "createOutline()" or "createOutlineFrom()" might be a fair replacement.

Hmmm... and if one took "createOutline()" as a name, then a choice to provide two functions might end up with "createOutline()" and then a second method named something like "createOutlineCoalesced()" or "createCoalescedOutline()" could be chosen.

So Krzysztof, given all of that, do you think this is a case where

Geom::PathVector *newThing = item_outline_coalesced(myItem);

is best or is it one where we'd want

Geom::PathVector *newThing = item_outline(myItem, true);

for getting the best code legibility?

Oh, and there is always the consideration of not using plain data types for flags, but having an explicit enum instead. That variation would give us

Geom::PathVector *newThing = item_outline(myItem, COALESCE);

On 04/01/2012 03:06, alvinpenner wrote:

thanks for the suggestions, I've made the call to Path::Coalesce optional. committed to rev 10835.

Initial tests with debug and optimized builds (both X11 and Quartz based) on two systems - Mac OS X 10.5.8, 32bit, 2.4 GHz Intel Core 2 Duo, 4GB RAM (upgraded) - OS X 10.7.2, 64bit, 2.4 GHz Intel Core i5, 8GB RAM show greatly improved performance when editing files with moderately complex stroked paths ('share/examples/l-systems.svgz' it a good test file IMHO: compare moving one of the generated paths - by dragging it with the mouse - in r10834 and r10835).

Debug builds (-g -O0) still noticeably lag compared to optimized builds (-O3), and the difference of performance between the two laptops (32bit vs 64bit ?) is quite big. But overall trunk is now much closer to the performance of current stable (if using an optimized build).

Many thanks @all for addressing this (r10835)!

I do want to ask about another (possibly related) performance issue which has been bothering me in trunk builds for quite some time and is still present in r10835 (note: performance of the same operations in stable 0.48.2 is ok and fast compared to trunk).

--> Spike of memory usage when zooming in closely in a plain document with lots of dashed stroked lines (e.g. a floor plan) (no filter effects, patterns, markers, etc. involved)

Please download the SVG file from http://commons.wikimedia.org/wiki/File:Zitouna_Mosque_plan.svg 1) open it in current trunk build of inkscape 2) enlarge the document window 3) zoom in repeatedly (to get below 100%)

At some point after changing the zoom levels a lot, Inkscape's memory usage explodes and it grabs whatever it can get (causes paging even with 8GB RAM), and it takes a long time until the window finally gets updated.

The memory (> 2.6 GB) is released immediately when opening a new window and closing the one with the mosque plan zoomed in (i.e. keeping the same inkscape process running, with a new, empty document).

I have this drastic performance loss when zooming in with such a sample file even when using optimized 64bit builds and 8GB RAM - on my older 32bit system, Inkscape usually stops responding, and I have to force-quit it (happens even after upgrading RAM from 2GB to 4GB).

If I edit the file and modify all objects (remove strokes or convert them to path, add fill as needed for tests), performance with zoom operations is ok -> this makes me think it is related to what is discussed in this thread. Possibly due to the many _dashed_ stroked lines?

Can anyone on linux/windows reproduce this?

~suv

2012/1/4 Nicolas Dufour <nicoduf@...48...>:

Confirmed on Windows XP, Inkscape revision 10822. The memory jumps from 70 Mio (initial document) to 1 Gio (I've stopped zooming at 800%...). Unzooming releases the memory. No unexpected memory consumption when the strokes are converted to path or removed, as you already noticed.

Does it still happen if you set the environment variable _INKSCAPE_DISABLE_CACHE to 1? e.g. in command prompt: set _INKSCAPE_DISABLE_CACHE=1 inkscape

Regards, Krzysztof

De : ~suv <suv-sf@...58...>

À : Krzysztof Kosiński <tweenk.pl@...400...>

...

Does it still happen if you set the environment variable _INKSCAPE_DISABLE_CACHE to 1? e.g. in command prompt: set _INKSCAPE_DISABLE_CACHE=1 inkscape

Testing with caching disabled (Inkscape 0.48+devel r10836 on OS X Lion 64bit, optimized build): zooming in and out works extremely fast and without any noticeable increase in memory usage.

Confirmed on Windows XP. -- Nicolas

On 4-1-2012 3:06, alvinpenner wrote:

thanks for the suggestions, I've made the call to Path::Coalesce optional. committed to rev 10835.

How about backporting this to 0.48.x to increase its speed? Do we dare do it? ;)

Ciao, Johan

On 25/01/2012 18:05, ~suv wrote:

On 04/01/2012 03:06, alvinpenner wrote:

...

thanks for the suggestions, I've made the call to Path::Coalesce optional. committed to rev 10835.

Related bug #906952 "Low performance while manipulating spirograph paths" [1] has been reopened today, based on newly provided example file [2] for which the fix in rev 10835 apparently doesn't help [3].

Steps to reproduce a similar loss of performance with basic Inkscape shapes:

1) delete/rename preferences file 2) launch inkscape (trunk) 3) draw three stars (use default settings, but increase number of corners to 200) 4) draw a rectangle

Test 1: 5) drag the rectangle with the mouse -> slight delay, depending on general system performance, but noticeably delayed compared to Inkscape 0.48.2

Test 2: 6) select the three stars, and change the stroke style to 'Round Join' and 'Round Cap' 7) drag the rectangle with the mouse -> noticeably (IMHO huge) loss of performance while dragging a simple shape

Note: the 'Spirograph' path in the second test file of the bug report doesn't even use rounded joins and caps.

hth, ~suv

[1] https://bugs.launchpad.net/bugs/906952 [2]

https://bugs.launchpad.net/inkscape/+bug/906952/+attachment/2692739/+files/drawing-spiro-test.svg

[3]

http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/10835

On 29-12-2011 0:38, Alvin Penner wrote:

 Attached is some timing information concerning the bounding box

calculation, using Inkscape rev 10794. For a test case I used the file spirograph.svg from LP Bug 906952. https://bugs.launchpad.net/inkscape/+bug/906952 The time delay appears to be located almost exclusively in the routine Path::Coalesce (file livarot\PathSimplify.cpp) which is called from the routine item_outline (file splivarot.cpp) which is called from the routine SPShape::sp_shape_bbox (file sp-shape.cpp) which may also optionally be called from the routine sp_group_bbox (file sp-item-group.cpp). On my machine, with this svg file, it takes 5 seconds to calculate the bounding box. The time required depends on the stroke. If I change the stroke from 0.5 px to 10 px, it takes only 1 second, but changing back from 10 px to 0.5 px takes 8 seconds, using the Fill and Stroke editor to modify stroke. In addition, the bounding box is often calculated twice, once if the object is selected, and once again in order to calculate the bbox of the whole image. Even if the complicated object is not selected, the bbox for it will still be calculated, no matter which object is selected or moved, as has been noted previously in this thread. This call appears to come from sp_group_bbox, as far as I can tell. In Inkscape 0.48.2, it appears that the bbox (with stroke) was calculated using the option SPItem::APPROXIMATE_BBOX, which no longer exists. This option compensated for stroke by adding half the stroke to the bbox at each side, but did not take into account the style of endpoints. It does not respond to any changes in EndCap style in the Fill and Stroke editor (but it is faaast). It appears that we need a compromise of sorts: if the path touches the (geometric) bbox tangentially, then we can ignore endcap style and use the half stroke width as in APPROXIMATE_BBOX (which is very fast), and if the path endpoint is touching the (geometric) bbox then we need to compensate for endcap style as is currently being done.

Keep in mind that there are cases where we really do not need to know the bbox very accurately, so it would be good to bring the approx bbox calculation back.

Ciao, Johan

On 10/10/2011 02:21 PM, LucaDC wrote:

Krzysztof Kosiński wrote:

...

Currently the visual bbox calculation is done in a very inefficient way using livarot. It could potentially be improved by using cairo_stroke_extents instead.

Thanks for all the replies/explanations. Ok, I'll positively wait for an improvement on this side. But maybe it's worth saying that I always use geometrical bounding boxes, so I can't understand why all these useless calculations on strokes are performed anyway (if I correctly understand).

What I believe that has happened is that someone introduced the geometric bounding box vs. visual bounding box option at some point in time, for a specific purpose. E.g. for allowing the user to specify dimensions of the geometric bounding box, whereas before Inkscape only had the visual bounding box. That's likely why it's a sub-option for the selector tool, and not a general option. Then, over time, more and more code was implemented that reads that parameter for other purposes, up to the point where we are now: some codes uses the visual vs. geom. bounding box parameter, whereas other code simply always uses the visual bounding box. That's probably related to what you're seeing and not what you weren't expecting.

Diederik