[Marble-devel] Review Request: add APRS data display to marble

[Wes Hardaker]

> On 2010-05-08 19:40:00, Torsten Rahn wrote:
> > trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.cpp, line 74
> > <http://reviewboard.kde.org/r/3843/diff/5/?file=25562#file25562line74>
> >
> >     I suggest you use 
> >     
> >     HOVERS_ABOVE_SURFACE 
> >     
> >     here instead of ALWAYS_ON_TOP. Otherwise your labels will get displayed too high and might be drawn in front of float items which will look odd.

Added.


> On 2010-05-08 19:40:00, Torsten Rahn wrote:
> > trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.cpp, line 346
> > <http://reviewboard.kde.org/r/3843/diff/5/?file=25562#file25562line346>
> >
> >     I suggest that you call 
> >     
> >     if ( renderPos != "HOVERS_ABOVE_SURFACE" ) {
> >         return true;
> >     }
> >     
> >     painter->save() 
> >     
> >     here. Saving and restoring the painter will avoid that other plugins get affected by your painter changes. The render position catch is just a safety measure ... 
> >

Added.


> On 2010-05-08 19:40:00, Torsten Rahn wrote:
> > trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.cpp, line 368
> > <http://reviewboard.kde.org/r/3843/diff/5/?file=25562#file25562line368>
> >
> >     Please call
> >     
> >     painter->restore();
> >     
> >     here. Otherwise all other plugins will be affected by your painter changes.

Added.  You know, I'd argue that the parent of the plugin should be calling this to prevent this sort of error.  Why make the plugin do it if every render plugin should be doing it?


- Wes


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


On 2010-05-04 02:54:40, Wes Hardaker wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3843/
> -----------------------------------------------------------
> 
> (Updated 2010-05-04 02:54:40)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> Adds support for displaying HAM radio aprs data to the map pulled from three different possible sources.  The TCP/IP source is the default that is enabled and usable by everyone.  The others would require special sources and default to off.  The icons, which are binary files, can be found at:  http://www.hardakers.net/code/marble/aprs-icons.tar.gz
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsFile.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsFile.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/data/CMakeLists.txt 1121250 
>   trunk/KDE/kdeedu/marble/src/plugins/render/CMakeLists.txt 1121250 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsConfigWidget.ui PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsGatherGen.pl PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsGatherer.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsGatherer.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsGatherer_mic_e.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsObject.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsPlugin.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsSource.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsSource.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTCPIP.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTCPIP.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTTY.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/AprsTTY.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/CMakeLists.txt PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/GeoAprsCoordinates.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/GeoAprsCoordinates.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/posix_qextserialport.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/qextserialport.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/qextserialport.cpp PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/qextserialport_global.h PRE-CREATION 
>   trunk/KDE/kdeedu/marble/src/plugins/render/aprs/win_qextserialport.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/3843/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> screenshot of california with data
>   http://reviewboard.kde.org/r/3843/s/378/
> 
> 
> Thanks,
> 
> Wes
> 
>