[Marble-devel] Review Request 110058: Spherical Projection Panning

Fri May 24 17:39:09 UTC 2013

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

This is coming together really nicely. Comments below mostly address code style wrt curly brackets.

src/lib/AbstractDataPlugin.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24465>

    curly brackets also for one-liners, please

src/lib/AbstractDataPlugin.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24466>

    curly brackets also for one-liners, please

src/lib/LayerInterface.h
<http://git.reviewboard.kde.org/r/110058/#comment24468>

    Please add
    Q_DISABLE_COPY(LayerInterface)

    (or implement copy and assignment operators that take care of d if Q_DISABLE_COPY is not possible)

src/lib/LayerInterface.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24467>

    delete d;

src/lib/LayerInterface.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24469>

    I tend to prefer isScreenStationary() as method name (see http://qt-project.org/wiki/API-Design-Principles)

src/lib/LayerManager.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24470>

    Curly brackets, please

src/lib/LayerManager.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24471>

    Curly brackets, please

src/lib/Projections/SphericalProjection.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24486>

    Curly brackets

src/lib/RenderPlugin.h
<http://git.reviewboard.kde.org/r/110058/#comment24472>

    Imho isScreenStationary here as well

src/lib/SphericalScanlineTextureMapper.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24473>

    Why is save()/restore() needed here? I don't see a change that alters painter state.

src/lib/SphericalScanlineTextureMapper.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24474>

    Curly brackets, please, or qMin()

src/lib/SphericalScanlineTextureMapper.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24475>

    Curly brackets or qMin()

src/lib/SphericalScanlineTextureMapper.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24476>

    Curly brackets

src/lib/SphericalScanlineTextureMapper.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24477>

    Curly brackets or qMin

src/lib/SphericalScanlineTextureMapper.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24478>

    Curly brackets or qMin()

src/lib/SphericalScanlineTextureMapper.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24479>

    Curly brackets

src/lib/TextureColorizer.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24480>

    Curly brackets

src/lib/TextureColorizer.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24481>

    Curly brackets or qMin()

src/lib/TextureColorizer.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24482>

    Curly brackets or qMin()

src/lib/TextureColorizer.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24483>

    Curly brackets

src/lib/TextureColorizer.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24484>

    Curly brackets or qMin()

src/lib/TextureColorizer.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24485>

    Curly brackets

src/lib/layers/VectorMapLayer.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24487>

    Curly brackets

src/lib/layers/VectorMapLayer.cpp
<http://git.reviewboard.kde.org/r/110058/#comment24488>

    Curly brackets

- Dennis Nienhüser

On May 15, 2013, 12:12 a.m., Paul Nader wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110058/
> -----------------------------------------------------------
> 
> (Updated May 15, 2013, 12:12 a.m.)
> 
> 
> Review request for Marble, Bernhard Beschow and Dennis Nienhüser.
> 
> 
> Description
> -------
> 
> Translates the scene viewpoint in a plane orthogonal to the line of sight of the viewer.
> 
> Version    Changes
> -------+-----------------------------------------------------------------
>    r6      Routing Layer now working :)
>            Reverted back to rendering while panning
>            Better overall FPS rate
>            Globe rotation adjusted while panning
>            Support for panning using keyboard and arrow disk widget arrows.
>            Bugfixes: https://github.com/oberluz/marble
> 
> 
> Diffs
> -----
> 
>   src/QtMainWindow.cpp 50a4676 
>   src/lib/AbstractDataPlugin.cpp 347f4b1 
>   src/lib/GoToDialog.cpp 57b65de 
>   src/lib/LayerInterface.h 46a20da 
>   src/lib/LayerInterface.cpp c3629de 
>   src/lib/LayerManager.cpp 0df19d2 
>   src/lib/MapInfoDialog.cpp 5219983 
>   src/lib/MarbleModel.h 44a5f61 
>   src/lib/MarbleModel.cpp a84ccae 
>   src/lib/MarbleWidget.h 9203250 
>   src/lib/MarbleWidget.cpp 3b5e844 
>   src/lib/MarbleWidgetInputHandler.cpp 1bdba92 
>   src/lib/PopupItem.cpp 20391fc 
>   src/lib/Projections/SphericalProjection.cpp cebb73d 
>   src/lib/RenderPlugin.h 90355ed 
>   src/lib/RenderPlugin.cpp b9b95c9 
>   src/lib/ScanlineTextureMapperContext.cpp ff3197b 
>   src/lib/SphericalScanlineTextureMapper.h 85ee814 
>   src/lib/SphericalScanlineTextureMapper.cpp 0bfe09c 
>   src/lib/TextureColorizer.cpp 5319c21 
>   src/lib/VectorMap.cpp cf3dcef 
>   src/lib/ViewportParams.h 6b97f55 
>   src/lib/ViewportParams.cpp f82a8b7 
>   src/lib/layers/VectorMapLayer.cpp f05c88a 
>   src/lib/routing/RoutingInputWidget.cpp 28db48d 
>   src/lib/routing/RoutingLayer.cpp 125990d 
>   src/plugins/render/aprs/AprsPlugin.cpp 550192f 
>   src/plugins/render/atmosphere/AtmospherePlugin.cpp 492c111 
>   src/plugins/render/earthquake/EarthquakePlugin.cpp 1205521 
>   src/plugins/render/navigation/ArrowDiscWidget.h 77e5cdd 
>   src/plugins/render/navigation/ArrowDiscWidget.cpp ee81025 
>   src/plugins/render/opencachingcom/OpenCachingComPlugin.cpp 576198f 
>   src/plugins/render/opendesktop/OpenDesktopPlugin.cpp b1cef50 
>   src/plugins/render/photo/PhotoPlugin.cpp cdb6243 
>   src/plugins/render/postalcode/PostalCodePlugin.cpp 87feacd 
>   src/plugins/render/stars/StarsPlugin.cpp d8d1a08 
>   src/plugins/render/weather/WeatherPlugin.cpp 4be0c5d 
>   src/plugins/render/wikipedia/WikipediaPlugin.cpp 89cf51e 
> 
> Diff: http://git.reviewboard.kde.org/r/110058/diff/
> 
> 
> Testing
> -------
> 
> Ubuntu 11.40 32-bit / Qt 4.8.1
> 
> 
> File Attachments
> ----------------
> 
> Panned
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/04/16/17._Satellites.png
> Design Notes
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/05/15/Design_Notes.txt
> 
> 
> Thanks,
> 
> Paul Nader
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20130524/1953b884/attachment-0001.html>