2Geom sync status

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

2Geom sync status

Krzysztof Kosiński
The 2Geom sync is almost done. So far I have identified two problem areas.

2Geom is now a lot more strict about what it considers a degenerate
closing segment - it requires exact equality of endpoints, not just
nearness to some arbitrary precision. This is of particular importance
in paths where the last segment is curved. Because Inkscape loses
precision when storing paths, bug #515237 now manifests on every
closed path with a non-linear last segment.

https://bugs.launchpad.net/inkscape/+bug/515237

I intend to fix this in the following way: first, modify SVGPathParser
so that it knows whether the last segment before 'z' was relative, and
if it is, snap it to the final node with a precision set by another
function call, 1e-6 by default; second, by making Inkscape output
paths with the double-conversion library now integrated in 2Geom and
always using absolute coordinates, so that there is perfect roundtrip
between XML and internal representation. This will mean the removal of
the Output Precision setting and a moderate increase in file sizes.
The proper place for such functionality is in a separate export
filter. As a bonus, setting the document unit to metres will no longer
cause a variety of issues.

We discussed output precision briefly on the hackfest. Moving to full
precision output with perfect roundtrip is the only correct solution
and will enable us to avoid excessive recomputation of attributes in
the SP tree, which is a performance win. If someone wants to get rid
of extra precision - use an export filter. However, if anyone has a
different opinion please speak up now.

The second problem is the transformation of elliptical arcs, as seen
when manipulating ellipses in the selector tool. In some cases, the
large arc / sweep flags are incorrectly set for transformed arcs. I am
still looking for the cause of this bug.

The sync branch is at lp:~inkscape.dev/inkscape/lib2geom-sync - if
anyone feels like looking for additional bugs, please do. I only did
minimal testing on it.

Regards, Krzysztof

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Josh Andler


On May 7, 2015 10:18 PM, "Krzysztof Kosiński" <[hidden email]> wrote:
> We discussed output precision briefly on the hackfest. Moving to full
> precision output with perfect roundtrip is the only correct solution
> and will enable us to avoid excessive recomputation of attributes in
> the SP tree, which is a performance win. If someone wants to get rid
> of extra precision - use an export filter. However, if anyone has a
> different opinion please speak up now.

My opinion is in line with yours. I will however say if removing the precision preference, we need to make sure the export filter exists by the next release.

> The second problem is the transformation of elliptical arcs, as seen
> when manipulating ellipses in the selector tool. In some cases, the
> large arc / sweep flags are incorrectly set for transformed arcs. I am
> still looking for the cause of this bug.

What's the bug report number on this one?

Cheers,
Josh


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Josh Andler
In reply to this post by Krzysztof Kosiński

On May 7, 2015 10:18 PM, "Krzysztof Kosiński" <[hidden email]> wrote:
>
> The 2Geom sync is almost done.

As soon as I it sent I realized the most important response was missing...  Yay! That is great news! :-)

Cheers,
Josh


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Krzysztof Kosiński
In reply to this post by Josh Andler
2015-05-08 15:20 GMT+02:00 Josh Andler <[hidden email]>:
>> The second problem is the transformation of elliptical arcs, as seen
>> when manipulating ellipses in the selector tool. In some cases, the
>> large arc / sweep flags are incorrectly set for transformed arcs. I am
>> still looking for the cause of this bug.
>
> What's the bug report number on this one?

Fixed that one a few minutes ago. I made two mistakes when simplifying
the ellipse transform function in 2Geom, and there are no unit tests
for ellipses yet...

At this point there are no blatantly obvious regressions in the sync
branch, but my testing was rather limited - I only tested things which
were likely to break based on the adjustments I made to Inkscape code.
In particular, EMF / WMF import and export and some LPEs need to be
checked.

Regards, Krzysztof

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Josh Andler
On Fri, May 8, 2015 at 6:57 AM, Krzysztof Kosiński <[hidden email]> wrote:
> At this point there are no blatantly obvious regressions in the sync
> branch, but my testing was rather limited - I only tested things which
> were likely to break based on the adjustments I made to Inkscape code.
> In particular, EMF / WMF import and export and some LPEs need to be
> checked.

So... check it out and bang on stuff. Got it! :)

Cheers,
Josh

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Josh Andler
I'm not sure if it's out of sync with trunk, but while messing around
it crashes if I try to use the eraser tool on a closed path.

The taper stroke LPE is crashing for me too on a converted star object.

I'll hopefully be able to test more today.

Cheers,
Josh

On Fri, May 8, 2015 at 7:01 AM, Josh Andler <[hidden email]> wrote:

> On Fri, May 8, 2015 at 6:57 AM, Krzysztof Kosiński <[hidden email]> wrote:
>> At this point there are no blatantly obvious regressions in the sync
>> branch, but my testing was rather limited - I only tested things which
>> were likely to break based on the adjustments I made to Inkscape code.
>> In particular, EMF / WMF import and export and some LPEs need to be
>> checked.
>
> So... check it out and bang on stuff. Got it! :)
>
> Cheers,
> Josh

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Tavmjong Bah
In reply to this post by Krzysztof Kosiński
On Fri, 2015-05-08 at 07:17 +0200, Krzysztof Kosiński wrote:

> The 2Geom sync is almost done. So far I have identified two problem areas.
>
> 2Geom is now a lot more strict about what it considers a degenerate
> closing segment - it requires exact equality of endpoints, not just
> nearness to some arbitrary precision. This is of particular importance
> in paths where the last segment is curved. Because Inkscape loses
> precision when storing paths, bug #515237 now manifests on every
> closed path with a non-linear last segment.
>
> https://bugs.launchpad.net/inkscape/+bug/515237
>
> I intend to fix this in the following way: first, modify SVGPathParser
> so that it knows whether the last segment before 'z' was relative, and
> if it is, snap it to the final node with a precision set by another
> function call, 1e-6 by default; second, by making Inkscape output
> paths with the double-conversion library now integrated in 2Geom and
> always using absolute coordinates, so that there is perfect roundtrip
> between XML and internal representation. This will mean the removal of
> the Output Precision setting and a moderate increase in file sizes.
> The proper place for such functionality is in a separate export
> filter. As a bonus, setting the document unit to metres will no longer
> cause a variety of issues.
>
> We discussed output precision briefly on the hackfest. Moving to full
> precision output with perfect roundtrip is the only correct solution
> and will enable us to avoid excessive recomputation of attributes in
> the SP tree, which is a performance win. If someone wants to get rid
> of extra precision - use an export filter. However, if anyone has a
> different opinion please speak up now.

It's not clear to me what the plan is for enabling an export filter. We
definitely need to offer our users the ability to limit the precision of
the output so before we store double precision numbers we need to have
this output filter in place. In addition, the ability to save as
relative coordinates is important (I use it all the time and it has been
requested by others; on the other hand, saving mixed absolute and
relative coordinates to save a few bytes is probably not very useful).

Recall, the SVG 2 spec now allows 'z' to fill in missing points at the
end of the path with the value of the first point so there is no need to
create a 'zero' length line segment to close a path due to rounding
errors. We should be able to parse this right away. Writing out this way
should probably wait until the other renderers have caught up.

Tav




------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Krzysztof Kosiński
2015-05-08 18:04 GMT+02:00 Tavmjong Bah <[hidden email]>:
> It's not clear to me what the plan is for enabling an export filter. We
> definitely need to offer our users the ability to limit the precision of
> the output so before we store double precision numbers we need to have
> this output filter in place. In addition, the ability to save as
> relative coordinates is important (I use it all the time and it has been
> requested by others; on the other hand, saving mixed absolute and
> relative coordinates to save a few bytes is probably not very useful).

I can do some more work on Geom::SVGPathWriter so that it can be
configured to do these things.

The export filter could be implemented as a visitor that creates a
separate XML document and populates it by calling write() method on
the SP tree. I'll look into this before changing over to full
precision.

> Recall, the SVG 2 spec now allows 'z' to fill in missing points at the
> end of the path with the value of the first point so there is no need to
> create a 'zero' length line segment to close a path due to rounding
> errors. We should be able to parse this right away. Writing out this way
> should probably wait until the other renderers have caught up.

Right now I 'snap' the final node to the initial node of the path when
the initial moveto or the last segment before 'z' is given in relative
coordinates. If both are given in absolute coordinates, they will
maintain exact coincidence despite rounding and therefore no snapping
is required.

Even when we move to extended 'z', the mandatory linear closing
segment remains useful, because it ensures that the size of range
begin(), end_closed() is equal to the number of nodes, i.e. there are
as many nodes in the path as there are segments between begin(),
end_closed(). I will probably exploit this when adding node-centric
path operations to 2Geom. These operations are interactive, so they
can be O(N) since rendering is also at the very least O(N).

PS: should the path parser in 2Geom have SVG 1.1 and SVG 2 versions,
or just a single version that accepts everything? Is it useful to
report errors for SVG 2 commands that are not in SVG 1.1?

Regards, Krzysztof

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Tavmjong Bah
On Fri, 2015-05-08 at 19:45 +0200, Krzysztof Kosiński wrote:

> 2015-05-08 18:04 GMT+02:00 Tavmjong Bah <[hidden email]>:
> > It's not clear to me what the plan is for enabling an export filter. We
> > definitely need to offer our users the ability to limit the precision of
> > the output so before we store double precision numbers we need to have
> > this output filter in place. In addition, the ability to save as
> > relative coordinates is important (I use it all the time and it has been
> > requested by others; on the other hand, saving mixed absolute and
> > relative coordinates to save a few bytes is probably not very useful).
>
> I can do some more work on Geom::SVGPathWriter so that it can be
> configured to do these things.

Excellent

> The export filter could be implemented as a visitor that creates a
> separate XML document and populates it by calling write() method on
> the SP tree. I'll look into this before changing over to full
> precision.
>
> > Recall, the SVG 2 spec now allows 'z' to fill in missing points at the
> > end of the path with the value of the first point so there is no need to
> > create a 'zero' length line segment to close a path due to rounding
> > errors. We should be able to parse this right away. Writing out this way
> > should probably wait until the other renderers have caught up.
>
> Right now I 'snap' the final node to the initial node of the path when
> the initial moveto or the last segment before 'z' is given in relative
> coordinates. If both are given in absolute coordinates, they will
> maintain exact coincidence despite rounding and therefore no snapping
> is required.

> Even when we move to extended 'z', the mandatory linear closing
> segment remains useful, because it ensures that the size of range
> begin(), end_closed() is equal to the number of nodes, i.e. there are
> as many nodes in the path as there are segments between begin(),
> end_closed(). I will probably exploit this when adding node-centric
> path operations to 2Geom. These operations are interactive, so they
> can be O(N) since rendering is also at the very least O(N).

OK, but we need to be careful about how markers are drawn. Having or not
having a small closing segment can effect which markers are drawn where.

> PS: should the path parser in 2Geom have SVG 1.1 and SVG 2 versions,
> or just a single version that accepts everything? Is it useful to
> report errors for SVG 2 commands that are not in SVG 1.1?

Just one version. It would be useful to report errors. There are a
couple of new path commands 'r' and 'b' but I doubt anybody will
implement them soon.

Tav



------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Krzysztof Kosiński
2015-05-08 20:18 GMT+02:00 Tavmjong Bah <[hidden email]>:
>> PS: should the path parser in 2Geom have SVG 1.1 and SVG 2 versions,
>> or just a single version that accepts everything? Is it useful to
>> report errors for SVG 2 commands that are not in SVG 1.1?
>
> Just one version. It would be useful to report errors. There are a
> couple of new path commands 'r' and 'b' but I doubt anybody will
> implement them soon.

So e.g. the path parser accepts everything but you can ask it whether
what was read was SVG 1.1 compliant?

Regards, Krzysztof

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Krzysztof Kosiński
In reply to this post by Josh Andler
2015-05-08 17:53 GMT+02:00 Josh Andler <[hidden email]>:
> I'm not sure if it's out of sync with trunk, but while messing around
> it crashes if I try to use the eraser tool on a closed path.

Now fixed, but the eraser tool is REALLY badly written...

- it creates a new XML object for the subtraction shape
- it tries to create a "stroke" around the mouse path, but does it
rather poorly, and the caps are not even approximately correct
- it has a pre-existing bug where erasing a rectangle sometimes won't work

Some of these could be fixed by rewriting it to use the path outliner,
but I'm not sure how robust that code is.

Regards, Krzysztof

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

liamw

On May 9, 2015 11:30 AM, "Krzysztof Kosiński" <[hidden email]> wrote:
>
> Some of these could be fixed by rewriting it to use the path outliner,
> but I'm not sure how robust that code is.
>
> Regards, Krzysztof
>

Why don't we make the change and find out ;)


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Reply | Threaded
Open this post in threaded view
|

Re: 2Geom sync status

Krzysztof Kosiński
2015-05-09 19:27 GMT+02:00 Liam White <[hidden email]>:
> On May 9, 2015 11:30 AM, "Krzysztof Kosiński" <[hidden email]> wrote:
>>
>> Some of these could be fixed by rewriting it to use the path outliner,
>> but I'm not sure how robust that code is.
>>
>> Regards, Krzysztof
>>
>
> Why don't we make the change and find out ;)

I'd rather keep the delta between the 2Geom sync branch and the trunk
reasonably small. Further improvements can be made after the sync
branch is merged.

Regards, Krzysztof

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Inkscape-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/inkscape-devel