Re: improvements on ellipticalquadrant in enhanced path of custom shape

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx
<mailto:rb.henschel@xxxxxxxxxxx>> 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




_______________________________________________
LibreOffice mailing list
LibreOffice@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/libreoffice




[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux