Re: [Lib2geom-devel] BezierCurve fun :-(

[<J.B...@ew...>]

> -----Original Message-----
> From: Krzysztof Kosiński [mailto:twe...@gm...]
> Sent: 13 February 2011 12:55
> To: Engelen, J.B.C. (Johan)
> Cc: lib...@li...
> Subject: Re: [Lib2geom-devel] BezierCurve fun :-(
> 
> 2011/2/12  <J.B...@ew...>:
>
> For example: let line and cubic be of type
> > LineSegment and CubicBezier. Then,
> >   Curve *line_reverse = line.reverse();
> >   Curve *cubic_reverse = cubic.reverse();
> > The question is: what type is line_reverse, and what type is
> > cubic_reverse? It turns out that line_reverse is of type LineSegment*,
> > and cubic_reverse it *not* of type CubicBezier*, but BezierCurve*.
> > Similar for cubic.transform(), derivative(), etc.
> 
> This is an oversight, of course the returns should have the correct
> specialized types.

This would mean that we have to type special case methods for CubicBezier and QuadBezier. We loose the advantage of one piece of code for all orders.


> > Perhaps we should remove the definitions for Cubic and QuadBezier.
> > Instead of
> >   if  ( CubicBezier *cubic = dynamic_cast<CubicBezier*>(curve))
> > isn't it nicer to write
> >   if  ( curve.order() == 3)
> > ?
> 
> It's even better to define static functions or methods:
> is_bezier_curve, is_line_segment, is_quadratic_bezier,
> is_cubic_bezier. For API orthogonality, the same static functions
> could be defined for other curve types: is_sbasis_curve,
> is_elliptical_arc.

Agreed.

> > 1a. If not, then why don't we delete these classes? This makes it
> much
> > clearer for users.
> 
> Deleting LineSegment actually makes the library less clear as
> described before (not everyone knows that a line segment is a
> first-order Bezier), so I would try to preserve at least LineSegment.
> 
> > 2. Is it correct that we should demote the use of dynamic_cast and
> > promote the use of order()==3?
> 
> dynamic_cast didn't work for me when I fixed the Node tool to work
> with the new 2Geom BezierCurve class. I think it's best to define the
> is_some_curve_type static functions to avoid his sort of question.

dynamic_cast does work, but instances of CubicBezier have a limited life-time, because simply doing a curve.reverse() makes it a BezierCurve instead.

 
> > What was the reason to switch from the templated BezierCurve (i.e.
> > typedef BezierCurve<3> CubicBezier) to what we have now?
> > It seems to have created quite a mess :-(
> 
> The old code made it impossible to refer to a Bezier curve of
> arbitrary order in a generic way. If you wanted to precess a path
> segment by segment, you had to have dynamic_cast switches for every
> possible order of Bezier curve you could encounter; in effect, every
> order of a Bezier behaved like a different class that had nothing in
> common with other orders, which is a very bad abstraction.

See my other mail. I think a better solution is to go back to BezierCurve<order>, and derive that one from BezierCurveGeneral for the handful of special general functions to beziers. 

> in effect, every
> order of a Bezier behaved like a different class that had nothing in
> common with other orders, which is a very bad abstraction. 

Note that the most used methods for beziers are already part of the Curve definition. So the classes did behave like they had most of their functionality in common. Still I can see that deriving them from a general BezierCurveGeneral class is useful.

Ciao,
  Johan