Review Request: KStars: fixed code checker issues #1

[Kevin Krammer]

> On Jan. 4, 2013, 6:17 p.m., Akarsh Simha wrote:
> > kstars/ekos/guide/matr.cpp, line 16
> > <http://git.reviewboard.kde.org/r/108166/diff/4/?file=104666#file104666line16>
> >
> >     Shouldn't math.h go below vect.h?
> >

I would have suggested the same, but lots of other files seem to suggest that there is a policy of including system headers before local headers


> On Jan. 4, 2013, 6:17 p.m., Akarsh Simha wrote:
> > kstars/skyglpainter.cpp, line 384
> > <http://git.reviewboard.kde.org/r/108166/diff/4/?file=104679#file104679line384>
> >
> >     After making this change, check that there are no issues with clipping of the sky polygons, and things like the Milky Way, horizon appear perfectly without problems in all projections.
> >     
> >     This might be introducing a bug. It is better not to assume that polygons are convex, although it does add some overhead in processing. MAKE_KSTARS_SLOW might make KStars a bit slow, but it might also fix a lot of bugs that we have to otherwise deal with.
> >     
> >     If you're unsure, I do not recommend making this change.

The change shouldn't change any runtime behavior at all.
The previous code defined MAKE_KSTARTS_SLOW which enabled the "then" branch of the ifdef.
This branch calls drawPolygon(polygon, false)

The new define's value is false, so the resulting call is again drawPolygon(polygon, false)
The idea was to remove the need for different code blocks but still keep the option of assuming convexity if desired.
We could improve the comment to say something like
// false -> slower but always accurate, true -> faster but could result in broken rendering

Krazy triggers on the define line because of the all-uppercase TRUE, so alternatively we could keep the ifdef as is and just remove the TRUE


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108166/#review24683
-----------------------------------------------------------


On Jan. 4, 2013, 5:44 p.m., Mohammed Nafees wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108166/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 5:44 p.m.)
> 
> 
> Review request for KDE Edu and Kevin Krammer.
> 
> 
> Description
> -------
> 
> http://www.google-melange.com/gci/task/view/google/gci2012/8203202
> 
> 
> Diffs
> -----
> 
>   Tests/CMakeLists.txt dfaa9d9 
>   Tests/testfwparser.cpp fc7e83a 
>   datahandlers/CMakeLists.txt a2d3207 
>   kstars/ekos/capture.cpp 59f203b 
>   kstars/ekos/ekosmanager.cpp 18b8b29 
>   kstars/ekos/guide.cpp 0aca5e9 
>   kstars/ekos/guide/common.cpp c3a7c08 
>   kstars/ekos/guide/gmath.cpp 042add9 
>   kstars/ekos/guide/guider.cpp aa85171 
>   kstars/ekos/guide/matr.cpp 25b6b68 
>   kstars/ekos/guide/rcalibration.h 268a49e 
>   kstars/ekos/guide/rcalibration.cpp 3787768 
>   kstars/ekos/guide/vect.cpp 9df115a 
>   kstars/fitsviewer/fitsviewer.cpp 33c2079 
>   kstars/oal/scope.cpp 819bdc0 
>   kstars/options/opssupernovae.cpp 970f116 
>   kstars/printing/legend.cpp 2eddd3c 
>   kstars/printing/printingwizard.cpp cd75b58 
>   kstars/skycomponents/asteroidscomponent.cpp b590dfb 
>   kstars/skycomponents/satellitescomponent.cpp 6585d07 
>   kstars/skycomponents/supernovaecomponent.h f2af09b 
>   kstars/skycomponents/supernovaecomponent.cpp 1c5a34d 
>   kstars/skyglpainter.cpp 22ae124 
>   kstars/skymapqdraw.cpp a495634 
>   kstars/skyobjects/satellite.cpp 770439f 
> 
> Diff: http://git.reviewboard.kde.org/r/108166/diff/
> 
> 
> Testing
> -------
> 
> built and tested
> 
> 
> Thanks,
> 
> Mohammed Nafees
> 
>

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