improvements on ellipticalquadrant in enhanced path of custom shape

Regina Henschel rb.henschel at t-online.de
Sat Dec 15 17:31:03 UTC 2018


Hi Noel, hi Armin, hi all,

my new solution is in https://gerrit.libreoffice.org/#/c/65203/

Noel Grandin schrieb am 09-Dec-18 um 07:42:
>
>
> On Sat, 8 Dec 2018 at 20:25, Regina Henschel <rb.henschel at t-online.de
> <mailto:rb.henschel at t-online.de>> wrote:
>
>     Problem A
>     The current implementation has a method GetPoint, which returns a
>     tools::Point; and such has integer coordinates. This introduces
>
>
> I would say you should change GetPoint() to return a basegfx::B2Point.
> You can't have two methods with the same name with different return
> types, and this is a place where it clearly should be returning a more
> accurate result.

I have extracted that calculation, which is already in double into a new 
GetPointAsB2DPoint. I have changed GetPoint to use this new function and 
only cast to Point in a last step. That allows to use GetPoint parallel 
until all callers have been changed to use double.

I have seen some other places with unnecessary double->long->double 
conversion, which can use the new function too. But that is not included 
in this patch.

>
>     Problem B
>     to createPolygonFromEllipseSegment to allow generating of
>     counter-clockwise arcs. Which way to go?
>
> You should add a parameter here. Either a boolean or an enum param,
> something like 'enum class Direction { Clockwise, CounterClockwise }'

I have talked to Armin. He points me to the method flip() to change the 
orientation of the generated polygon.

>
>
>
>     Problem C
>     the path is ill-structured and the first parameter point is consumed by
>     moveTo? Keep the direction or toggle it?
>
> With ill-structured data, I would say that you should just 'not crash'
> and error out at the earlier opportunity. There is no need to try doing
> something sensible with such data unless necessary for some kind of
> compaibility.

I have kept the old behavior, only taken the special cases out of the loop.

>
>
>     Problem D
>     How to make a unit test for such patches
>     (https://gerrit.libreoffice.org/#/c/64704/1 is another one)? It would
>     need to compare a bad rendering with a correct one and it would need to
>
>
> For this kind of thing, one approach is to query the output
> programmatically to see that it is correct. Other times we dump the
> output using dumpAsXml() and query the output using XPath.

I use now the bounding rectangle to distinguish between correct and bad 
behavior. If I turn a quarter circle by 45deg, I get an arc, which is 
edge of a segment with horizontal chord. If the curve is not an arc from 
a circle, that results in a different height of the segment and because 
of the 45deg rotation directly in a different height of the bounding 
rectangle.

Perhaps you (or someone else) have some time to look at my patch?

Kind regards
Regina






More information about the LibreOffice mailing list