Review Request 127354: improve drawing precision for circles

[Maurizio Paolini]

> On March 19, 2016, 5:04 p.m., David Narváez wrote:
> > Thanks for the patch! I would be in favor of dropping the QRect/QPoint usage in these files completely. Could you modify your patch to see how that would look like?
> 
> Maurizio Paolini wrote:
>     After some trying around I realize that dropping QRect in (e.g.) the KigPainter class cannot be done easily.  In particular one should
>     understand the "overlay" concept... If I remember, at least in the early stages of kig development, overlays where used in order to
>     allow for efficent redrawing of the graphic primitives.  I am not even sure that this is still used now!  This was taken from the kgeo
>     project when first starting kig.  If I place an "assert (false)" in "circleOverlayRecurse", then kig crashes... however I don't see the
>     need of using overlays for drawing circles if we use a Qt primitive for doing that.
> 
> David Narváez wrote:
>     Interesting, I will check that. I did a quick pass through the code last weekend and was under the impression that there are needs for QRect in several places but KigPainter could do without QRect, but I must have missed this.
> 
> Maurizio Paolini wrote:
>     Now, if you ask me, I have the strong suspect that overlays are not used anymore... If this is the case, there is plenty of dead code in kigpainter.  For example I tried to comment out the line
>        if( mNeedOverlay ) circleOverlay( center, radius );
>     in drawCircle and nothing (apparently) happened.  However who knows...

So this is my suggestion:
- commit this small patch keeping it minimal
- evaluate the whole "overlay" affair (under the suspect that it is no longer used and some code overhead could be dropped)
- reconsider removal of QRect/QPoint in favour of QRectF/QPointF in kigpainter once the mOverlay member is no longer present

I recall that overlays where a mean to improve dynamic drawing and avoid flickering.  In kig it was reportedly imported by
Dominique Devriese from the previous "kgeo" project, now defunct (by Marc Bartsch), and also had some contribution by me in the
drawing of circles part (circleOverlay).
If someone has better knowledge of overlays, please contribute!


- Maurizio


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


On March 19, 2016, 9:05 a.m., Maurizio Paolini wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127354/
> -----------------------------------------------------------
> 
> (Updated March 19, 2016, 9:05 a.m.)
> 
> 
> Review request for KDE Edu, David Narváez and Rex Dieter.
> 
> 
> Bugs: 359805
>     http://bugs.kde.org/show_bug.cgi?id=359805
> 
> 
> Repository: kig
> 
> 
> Description
> -------
> 
> QRect object in qt uses integers.  This causes annoying loss of drawing precision when drawing circles (https://bugs.kde.org/show_bug.cgi?id=359805).  Using the QRectF object seems more appropriate.
> 
> 
> Diffs
> -----
> 
>   misc/kigpainter.h e36c153 
>   misc/kigpainter.cpp e547bf2 
>   misc/screeninfo.h b7f94c4 
>   misc/screeninfo.cc 8fdf115 
> 
> Diff: https://git.reviewboard.kde.org/r/127354/diff/
> 
> 
> Testing
> -------
> 
> This is a minimal patch to allow using QRectF when drawing circles.  Perhaps using QRect
> (and QPoint) could be dropped completely in favour of QRectF and QPointF.
> 
> 
> File Attachments
> ----------------
> 
> tangent circles - old behaviour
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/19/48f5447f-0904-43ee-8c56-8fc07c03febb__circles_old.png
> tangent circles: new behaviour
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/19/5684b1c5-c32c-447b-bed6-53be380f1b87__circles_new.png
> zoom of old behaviour
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/19/08937b54-e313-4acf-b482-4bd899d24b30__circles_old_zoom.png
> zoom of new behaviour
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/19/e7c0b8fb-af81-43da-b70e-bed3462d73a6__circles_new_zoom.png
> 
> 
> Thanks,
> 
> Maurizio Paolini
> 
>

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