[Marble-devel] Re: Review Request: Custom position marker rotation

[Dennis Nienhüser]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6280/#review9528
-----------------------------------------------------------


Looks good, some minor things below.


/trunk/KDE/kdeedu/marble/src/plugins/render/positionmarker/PositionMarker.cpp
<http://svn.reviewboard.kde.org/r/6280/#comment10530>

    Please use M_PI.
    



/trunk/KDE/kdeedu/marble/src/plugins/render/positionmarker/PositionMarker.cpp
<http://svn.reviewboard.kde.org/r/6280/#comment10531>

    Can you make the SmoothTransformation parameter depend on the on the quality settings of the painter? Use SmoothTransformation for high and print quality, otherwise not.
    
    Are you rotating using the correct origin point? IIRC this call rotates around the top left point, while I'd say you should rotate around the image center. If that's the case, use something like 
    transform.translate( -w/2, -h/2 )
    transform.rotate( m_heading )
    transform.translate( w/2, h/2 )
    



/trunk/KDE/kdeedu/marble/src/plugins/render/positionmarker/PositionMarkerConfigWidget.ui
<http://svn.reviewboard.kde.org/r/6280/#comment10532>

    What about
    "Position marker images indicating the direction should point north (top)." to also cover the case where the image has no direction (e.g. a circle).


- Dennis


On 2011-01-05 14:33:36, Daniel Marth wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6280/
> -----------------------------------------------------------
> 
> (Updated 2011-01-05 14:33:36)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> Custom position markers are now rotated according to the heading.
> GCI-task: http://www.google-melange.com/gci/task/show/google/gci2010/kde/t129417733003
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/data/CMakeLists.txt 1212016 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/positionmarker/PositionMarker.h 1212016 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/positionmarker/PositionMarker.cpp 1212016 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/positionmarker/PositionMarkerConfigWidget.ui 1212016 
> 
> Diff: http://svn.reviewboard.kde.org/r/6280/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Daniel
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20110105/a2eccc6d/attachment.htm