[Marble-devel] Review Request 124493: Showing location in Marble Maps

[Gábor Péterffy]

> On júl. 28, 2015, 9:47 de, Dennis Nienhüser wrote:
> > src/lib/marble/MarbleQuickItem.cpp, line 270
> > <https://git.reviewboard.kde.org/r/124493/diff/3/?file=388181#file388181line270>
> >
> >     There's an unfortunate trap hidden in Marble's plugin implementation: A Plugin is both a factory of a certain interface and an implementation of that interface. Therefore a RenderPlugin is a factory for a LayerInterface (you can use it to create something that draws on the map) and an implementation of LayerInterface (you can use it directly to draw on the map).
> >     
> >     The desired usage is that PluginManager holds the factory part of the plugins, and anything that wants to use an instance of the corresponding interface retrieves the factory (the plugin) from PluginManager and calls its factory method `newInstance()`.
> >     
> >     The trap then is that all the plugins that PluginManager holds look and behave like implementing some interface, but in reality the instances that are actually used lie elsewhere. We will fix that at some point in the future (e.g. `AbstractFloatItem` will not inherit `RenderPlugin`, but `RenderPluginInterface`), but for now have to take care manually.
> >     
> >     Long story short: Please iterate over `map()->renderPlugins()` here, not `model()->pluginManager()->renderPlugins()`. Same for `setShowPositionMarker` below.

Thank you for pointing out this for me. I have not noticed this until now.


- Gábor


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


On júl. 28, 2015, 10:35 de, Gábor Péterffy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124493/
> -----------------------------------------------------------
> 
> (Updated júl. 28, 2015, 10:35 de)
> 
> 
> Review request for Marble.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> I have updated the qtmobility plugin to be able to compile with Qt5, and I also enabled the PositionMarker render plugin for Marble Maps.
> 
> Focusing to the current position will be in a new patch. Also I think the current position marker has to be redesigned for the osm theme because now it is hiding (see the screenshot).
> 
> 
> Diffs
> -----
> 
>   src/apps/marble-maps/MainScreen.qml 5552a54eca8b37fa17588a14a035927418b23fbe 
>   CMakeLists.txt e74694a0da8507415a9a5472c4f09dfe2561f846 
>   src/apps/marble-maps/CMakeLists.txt fff00ffc5ee4dcb3a78b5cdea18035e9267df1e9 
>   src/apps/marble-maps/create-apk.py f5fa5489aae2780b04f61b32b21e4c45bd3df85b 
>   src/apps/marble-maps/package/AndroidManifest.xml 3eee53b142c444a3f456208220b3e91ef5759219 
>   src/lib/marble/MarbleQuickItem.h 21b8fe5c4570ac894f668603a660da81f1d8a8e4 
>   src/lib/marble/MarbleQuickItem.cpp ee8bae8ea379cae3a0e6378259622a1ad88f8a2b 
>   src/plugins/positionprovider/CMakeLists.txt f9bfdd691d1905db29cad41af769320c694cd324 
>   src/plugins/positionprovider/qtmobility/CMakeLists.txt 502513afef8f9bb9b23d70eb868c7a751c7fc4d6 
>   src/plugins/positionprovider/qtmobility/QtMobilityPositionProviderPlugin.h edf75285c18473a42f0bc7dd0c406206626a31fc 
>   src/plugins/positionprovider/qtmobility/QtMobilityPositionProviderPlugin.cpp a2e6dca574564938813ed06466bf67b1a9900fcb 
>   src/plugins/positionprovider/qtpositioning/CMakeLists.txt PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124493/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> Screenshot
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/27/22c65232-7061-4ec5-86f1-de4acbd95e05__Screenshot_2015-07-27-19-04-35.png
> 
> 
> Thanks,
> 
> Gábor Péterffy
> 
>

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