-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor GUI map preview #1385
Merged
Merged
Refactor GUI map preview #1385
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this allows handling errors parsing the gpx file. this allows GMapDlg::gpx_ to be const.
Wow. Thanks!
…On Sun, Nov 17, 2024, 1:17 PM tsteven4 ***@***.***> wrote:
Among other cleanups this enforces storage of which map items are visible
to GMapDlg::model_. Duplication of this information, for example in
GpxItem::visible, has been eliminated. This allows simplification as we can
often use common routines passing a parent or child of the model instead of
separate routine for wayoints/tracks/routes. It also improves
maintainability and correctness as the possibility of disagreement is
eliminated (which could occur previously). It allows GMapDlg::gpx_ to be
const, which ensures we aren't operating on copies of information, as we
were in GMapDialog::[show|hide[AllWaypoints|Track|Routes (although the
code worked anyway, the operation on the temporary copy was just
obfuscation. Overall we are down 347 LOCs.
This started as remove of foreach. That is readily achievable in the GUI,
but fraught with peril in the CLI due to routines that operate on global
lists that you may be iterating over.
I still need to turn debug off before merging.
------------------------------
You can view, comment on, or merge this pull request online at:
#1385
Commit Summary
- 308dc41
<308dc41>
refactor map preview
- 35ac695
<35ac695>
dont use QStandardItem data.
- 8bdbaea
<8bdbaea>
consistent use of model to set visiblity.
- 8e7e96b
<8e7e96b>
parse gpx for map in mainwindow
- 758aee9
<758aee9>
remove now unnecessary as_const.
- ac709bd
<ac709bd>
make map debug optional.
- fe656c6
<fe656c6>
tidy up gpx.cc
- 3a2d6f3
<3a2d6f3>
ws
- c668ebf
<c668ebf>
use lambda to simplify context menu.
File Changes
(8 files <https://github.com/GPSBabel/gpsbabel/pull/1385/files>)
- *M* gui/gmapdlg.cc
<https://github.com/GPSBabel/gpsbabel/pull/1385/files#diff-1192e7cdfbc6b795af344098c255de63f0f21fd4102262e5b011b3cde1038970>
(434)
- *M* gui/gmapdlg.h
<https://github.com/GPSBabel/gpsbabel/pull/1385/files#diff-bd0f0c9d467f6dc0ee782ba5f66e43bc169dd62cc7b9e16d60c0c617f98a65d4>
(63)
- *M* gui/gpx.cc
<https://github.com/GPSBabel/gpsbabel/pull/1385/files#diff-c171ad0098ca937e6a75f1c817139c25ee626cc146e44a6c1e8f0e49fd35d44c>
(61)
- *M* gui/gpx.h
<https://github.com/GPSBabel/gpsbabel/pull/1385/files#diff-7abd84eede9ec6402d555ca82ed5057e6701984247cbb4afc6b1b50c7bb29999>
(141)
- *M* gui/latlng.h
<https://github.com/GPSBabel/gpsbabel/pull/1385/files#diff-83815bbebb4ee03651496b6a75512e884c1bddf81ad4bc3e244378190c1dfe20>
(6)
- *M* gui/mainwindow.cc
<https://github.com/GPSBabel/gpsbabel/pull/1385/files#diff-663cf27dd0e87e9ed4c2f180503f2bb726dd837aee969db6541d76af0af9504e>
(24)
- *M* gui/map.cc
<https://github.com/GPSBabel/gpsbabel/pull/1385/files#diff-eb734b2d2b78e0e2afb3b06b7c7a0ad289cf1ecee5fc2d3612c86487a9ce7d92>
(96)
- *M* gui/map.h
<https://github.com/GPSBabel/gpsbabel/pull/1385/files#diff-c278633a2060880ce7bc026adee6ff17f73c417a630167989024cf77abfabee3>
(40)
Patch Links:
- https://github.com/GPSBabel/gpsbabel/pull/1385.patch
- https://github.com/GPSBabel/gpsbabel/pull/1385.diff
—
Reply to this email directly, view it on GitHub
<#1385>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC3VAD432CB47LQWLMDPBK32BDTUZAVCNFSM6AAAAABR6HOTDGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGY3DMMZSG4YTMNQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
"Show Only This ..." will pan to the selected waypoint or frame the selected track or route. "Show All ..." will recenter the map and reset the bounds.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Among other cleanups this enforces storage of which map items are visible to GMapDlg::model_. Duplication of this information, for example in GpxItem::visible, has been eliminated. This allows simplification as we can often use common routines passing a parent or child of the model instead of separate routine for wayoints/tracks/routes. It also improves maintainability and correctness as the possibility of disagreement is eliminated (which could occur previously). It allows GMapDlg::gpx_ to be const, which ensures we aren't operating on copies of information, as we were in GMapDialog::[show|hide[AllWaypoints|Track|Routes (although the code worked anyway, the operation on the temporary copy was just obfuscation. Overall we are down 347 LOCs.
This started as remove of foreach. That is readily achievable in the GUI, but fraught with peril in the CLI due to routines that operate on global lists that you may be iterating over.
I still need to turn debug off before merging.