Review Request 127364: improving CircleCircleIntersection (towards what geogebra does)

[Maurizio Paolini]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127364/#review93643
-----------------------------------------------------------



Before committing I would like to have feedback about one choice that has to be decided carefully:

A) Substitute the present CircleBTPType and ArcBTPType with the new version.  This has the drawback of introducing a little
   backward compatibility risk:  The new version *could* open a previously saved kig file producing a different construction.
   In particular this could happen whenever a saved construction used one of the two intersection points of e.g. a CircleBTP
   and a line for further constructions, for example to build a locus.

B) Add a *new* CircleBTPoType and ArcBTPoType to be used in place of the old constructions in newly created kig documents. An old
   saved file would contain references to CircleBTPType and continue to work as before.  This has the drawback of adding complexity
   in kig.  This is the path that I followed in the path file for this review.

Actually I would prefer choice "A" over "B" (I don't think there is *any* kig save file around the world that would be impacted!),
anyway, suggestions are appreciated.

- Maurizio Paolini


On March 16, 2016, 8:08 a.m., Maurizio Paolini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127364/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 8:08 a.m.)
> 
> 
> Review request for KDE Edu, David Narváez and Rex Dieter.
> 
> 
> Repository: kig
> 
> 
> Description
> -------
> 
> I am addressing two related issues: intersection points that jump, circles degenerating to lines.
> When intersecting circles obtained as "circle by 3 points" it is possible that one or both degenerate into a straight line.  At present when dinamically moving one of the three defining points of a circle across the line through the other two, the two intersection points exchange position, possibly causing the subsequent constructions to change abruptly.
> The other related issue is that at the moment the intersection point is not defined when one
> of the two circles degenerate into a line.
> 
> Motivation: this situations naturally arise, for example, when trying to construct the Disk Poincare' model of the hyperbolic plane, in which case it is essential to allow for circles to become lines.
> 
> A sample kig file that exposes these issues is available as http://dmf.unicatt.it/~paolini/kig/bugs/test_circle_intersection_old.kig
> 
> The big black points can be moved across the other two defining one of the two circles, observe that the two intersection points exchange position.  Moreover, if a black point
> is precisely aligned with the other two (press the shift key to force the point
> onto the grid), then the two intersection points disappear.
> 
> 
> Diffs
> -----
> 
>   misc/builtin_stuff.cc e1de877 
>   misc/common.cpp 2e1fac9 
>   objects/arc_type.h a9a7296 
>   objects/arc_type.cc 26c05cc 
>   objects/circle_imp.h dc63e06 
>   objects/circle_imp.cc 7450ef6 
>   objects/circle_type.h 37cf400 
>   objects/circle_type.cc 25ec233 
>   objects/intersection_types.cc 2fd07be 
>   objects/other_imp.h 31feda6 
>   objects/other_imp.cc d773196 
> 
> Diff: https://git.reviewboard.kde.org/r/127364/diff/
> 
> 
> Testing
> -------
> 
> Unfortunately, to address the "abrupt intersection points exchange" it seems necessary to add a (hidden) circle orientation for CircleImp(s).  In the proposed patch this is achieved by allowing the "mradius" member to be negative (it is "private", so that we to not risk to expose this possibility; radius() method will now return the absolute value of mradius, and the new "orientation()" method returns the circle orientation).
> 
> The additional 'orientation' information can then be used inside the CircleCircleIntersection::calc to avoid the abrupt jump of the intersection points.
> 
> The new CircleBTPoType class constructs an "oriented" circle (note that the user will not
> be able to directly access the orientation information), whereas the old CircleBTPType is left unchanges.  CircleBTPoType substitutes CircleBTPType in builtin_stuff.cc.
> In this way we ensure backward compatibility for old saved kig files, whereas new constructions will take advantage of the orientation.
> 
> Alternatively we can simply substitute the present CircleBTP with the new version.
> 
> 
> File Attachments
> ----------------
> 
> old behaviour
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/13/31028aa6-140d-40c2-9a79-743ed2e10f40__test_circle_intersection_old.kig
> new behaviour
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/13/526e0448-2b20-454b-ae2a-a5537bb94e99__test_circle_intersection_new.kig
> 
> 
> Thanks,
> 
> Maurizio Paolini
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20160317/d94113fa/attachment.html>