----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124519/#review83334 -----------------------------------------------------------
We're nearly there. Mostly need to clarify the different signals and slots such that they behave as expected. src/apps/marble-maps/MainScreen.qml (line 73) <https://git.reviewboard.kde.org/r/124519/#comment57594> This is a bool property and the signal should only be emitted when its value changes from false to true or from true to false. When you change the positionAvailable() and updatePositionVisibility() implementations (see comments there), the update of the direction indicator will then stop working based on this signal (which is correct). Therefore we need another signal in MarbleQuickItem, e.g. viewportChanged, which can be directly connected to visibleLatLonAltBoxChanged from MarbleMap. Then this method can be triggered as `onViewportChanged: {...}`. src/lib/marble/declarative/MarbleQuickItem.h (line 169) <https://git.reviewboard.kde.org/r/124519/#comment57585> Signals should have a passive name, slots an active. That way code is easier to read. Let's call it `updatePositionVisibility` src/lib/marble/declarative/MarbleQuickItem.cpp (line 93) <https://git.reviewboard.kde.org/r/124519/#comment57586> This is not needed. No touch event will change the ego position, and all kinds of map panning and zooming will result in a visibleLatLonAltBoxChange signal. src/lib/marble/declarative/MarbleQuickItem.cpp (line 181) <https://git.reviewboard.kde.org/r/124519/#comment57587> should call updatePositionVisiblity instead (the current visibleLatLonAltBoxChanged slot) src/lib/marble/declarative/MarbleQuickItem.cpp (line 196) <https://git.reviewboard.kde.org/r/124519/#comment57588> should call updatePositionVisiblity instead (the current visibleLatLonAltBoxChanged slot) src/lib/marble/declarative/MarbleQuickItem.cpp (line 199) <https://git.reviewboard.kde.org/r/124519/#comment57592> This should be renamed updatePositionVisibility and have the current implementation of positionVisible() (see also comment there) src/lib/marble/declarative/MarbleQuickItem.cpp (line 341) <https://git.reviewboard.kde.org/r/124519/#comment57591> This should return d->m_positionVisible and do nothing else. Otherwise caching d->m_positionVisible makes no sense. src/lib/marble/declarative/MarbleQuickItem.cpp (line 343) <https://git.reviewboard.kde.org/r/124519/#comment57589> for code readability I'd prefer renaming it isVisible src/lib/marble/declarative/MarbleQuickItem.cpp (line 407) <https://git.reviewboard.kde.org/r/124519/#comment57584> Bounding boxes aren't the only thing we can center on. Just use `d->centerOn(coordinates);` as a replacement for the four lines here. - Dennis Nienhüser On Aug. 2, 2015, 3:36 p.m., Gábor Péterffy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124519/ > ----------------------------------------------------------- > > (Updated Aug. 2, 2015, 3:36 p.m.) > > > Review request for Marble. > > > Repository: marble > > > Description > ------- > > This patch introduces the CircleButton qml type. Based on this there is a > button now which navigates the map at the current position. > > > Diffs > ----- > > data/android/drawable-xxxhdpi/gps_not_fixed.png PRE-CREATION > data/android/drawable-xxxhdpi/marblelogo.png > bf462f7608169f70cc014d8da1e0ad86e11bed15 > src/apps/marble-maps/BoxedText.qml PRE-CREATION > src/apps/marble-maps/CircularButton.qml PRE-CREATION > src/apps/marble-maps/MainScreen.qml > ce36f04c61764c7d813608f3bb7aeb671731c5a4 > src/apps/marble-maps/MarbleMaps.qrc > 2978e72c2fd12fe8435cfb3d5e5559982b41b110 > src/apps/marble-maps/PositionButton.qml PRE-CREATION > src/lib/marble/declarative/MarbleQuickItem.h > cca329761f23cd72315df2d359166a2cbfa8056c > src/lib/marble/declarative/MarbleQuickItem.cpp > 28f599c3b51c7bf993e40c6c97265f351fe02f17 > > src/plugins/positionprovider/qtpositioning/QtPositioningPositionProviderPlugin.cpp > 1a11d67fba96f328a0a5aba7545fa4e93c597964 > data/android/drawable-xxxhdpi/backdrop.png PRE-CREATION > data/android/drawable-xxxhdpi/gps_fixed.png PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/124519/diff/ > > > Testing > ------- > > It seems my distance or angle functions are not working well, the almost > always provide false data after some panning (my position is sometimes start > to change when I am panning on the map, strange thing: the position marker > also starts to go to an other place like it is fixed to the screen, but the > map moves under it sometimes when I am penning and it is not jumping back to > its place.) > > > File Attachments > ---------------- > > gps_not_fixed.png > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/30/35409caf-2dbd-4fc4-b495-43fe1d8e4279__gps_not_fixed.png > gps_fixed.png > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/30/019abdbb-80cf-4935-8d6d-0c3e6927d2b3__gps_fixed.png > marblelogo.png > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/31/ebaa4d45-b337-4e42-8a23-a73e2d88ddca__marblelogo.png > backdrop.png > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/31/dacf5958-7270-40c4-9031-243350a80de8__backdrop.png > > > Thanks, > > Gábor Péterffy > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
