Gpx osmand color #5400

2023-06-27T19:54:53Z

cyber-toad commented

2023-06-27 19:54:53 +00:00

(Migrated from github.com)

Add color support for OSMAnd #5287

@biodranik - I noticed a strange thing - for kml also if we set up some hex color for waypoint (bookmark) it is displayed as red. I tested with Google Earth. Maybe it is because some pre-defined set of colors (available in choose color dialog) is used and hex are ignored?
But they are sucessfully parsed from bookmark file - see unit test.

Add color support for OSMAnd #5287 @biodranik - I noticed a strange thing - for kml also if we set up some hex color for waypoint (bookmark) it is displayed as red. I tested with Google Earth. Maybe it is because some pre-defined set of colors (available in choose color dialog) is used and hex are ignored? But they are sucessfully parsed from bookmark file - see unit test.

biodranik (Migrated from github.com) reviewed 2023-06-28 06:03:05 +00:00

data/gpx_test_data/osmand2.gpx

					
				@ -0,0 +22,4 @@

				    </extensions>

				  </wpt>

				  <wpt lat="52.2136024" lon="19.4585842">

				    <ele>111</ele>

biodranik (Migrated from github.com) commented

2023-06-28 05:59:27 +00:00

Is ele value extracted and later stored in kml?

kml/serdes_gpx.cpp

					
				@ -140,6 +140,39 @@ void GpxParser::ParseColor(std::string const & value)

				  m_color = kml::ToRGBA(colorBytes[0], colorBytes[1], colorBytes[2], (char)255);

biodranik (Migrated from github.com) commented

2023-06-28 06:01:18 +00:00

Is it possible to rewrite this block using switch?

Is it possible to rewrite this block using `switch`?

kml/serdes_gpx.hpp

biodranik (Migrated from github.com) commented

2023-06-28 06:03:01 +00:00

How it will be handled differently from m_color?

cyber-toad (Migrated from github.com) reviewed 2023-06-28 06:17:11 +00:00

kml/serdes_gpx.hpp

cyber-toad (Migrated from github.com) commented

2023-06-28 06:17:10 +00:00

It is not reset in GpxParser::ResetPoint

biodranik (Migrated from github.com) reviewed 2023-06-28 06:30:12 +00:00

kml/serdes_gpx.cpp

biodranik (Migrated from github.com) commented

2023-06-28 06:30:11 +00:00

Does it make sense to introduce kInvalidColor constant instead of using zeroes here and in other places?

cyber-toad (Migrated from github.com) reviewed 2023-06-28 20:58:08 +00:00

kml/serdes_gpx.cpp

cyber-toad (Migrated from github.com) commented

2023-06-28 20:58:08 +00:00

Fixed

cyber-toad (Migrated from github.com) reviewed 2023-06-28 21:57:39 +00:00

kml/serdes_gpx.cpp

					
				@ -140,6 +140,39 @@ void GpxParser::ParseColor(std::string const & value)

				  m_color = kml::ToRGBA(colorBytes[0], colorBytes[1], colorBytes[2], (char)255);

cyber-toad (Migrated from github.com) commented

2023-06-28 21:57:38 +00:00

Fixed

cyber-toad (Migrated from github.com) reviewed 2023-06-28 22:04:23 +00:00

data/gpx_test_data/osmand2.gpx

					
				@ -0,0 +22,4 @@

				    </extensions>

				  </wpt>

				  <wpt lat="52.2136024" lon="19.4585842">

				    <ele>111</ele>

cyber-toad (Migrated from github.com) commented

2023-06-28 22:04:22 +00:00

I noticed that we don't use ele for gpx, but looks like there is some place for it in kml: kml::ParsePointWithAltitude. I can raise an issue to support it in gpx parser, but how do we use it - I was not able to find it in OM?

biodranik (Migrated from github.com) reviewed 2023-06-29 05:31:29 +00:00

data/gpx_test_data/osmand2.gpx

					
				@ -0,0 +22,4 @@

				    </extensions>

				  </wpt>

				  <wpt lat="52.2136024" lon="19.4585842">

				    <ele>111</ele>

biodranik (Migrated from github.com) commented

2023-06-29 05:31:29 +00:00

It may not be used yet, but we need to support it in the future to get track info/stats and to export back the important elevation data. A separate issue would be great.

biodranik (Migrated from github.com) reviewed 2023-06-29 05:33:58 +00:00

kml/serdes_gpx.cpp

					
				@ -140,6 +140,39 @@ void GpxParser::ParseColor(std::string const & value)

				  m_color = kml::ToRGBA(colorBytes[0], colorBytes[1], colorBytes[2], (char)255);

				}

biodranik (Migrated from github.com) commented

2023-06-29 05:32:12 +00:00

kInvalidColor

kml/serdes_gpx.cpp

biodranik (Migrated from github.com) commented

2023-06-29 05:32:53 +00:00

Is it possible that value.size() can be 0, and - 1 will lead to overflow?

Is it possible that value.size() can be 0, and `- 1` will lead to overflow?

cyber-toad (Migrated from github.com) reviewed 2023-06-29 06:09:21 +00:00

data/gpx_test_data/osmand2.gpx

					
				@ -0,0 +22,4 @@

				    </extensions>

				  </wpt>

				  <wpt lat="52.2136024" lon="19.4585842">

				    <ele>111</ele>

cyber-toad (Migrated from github.com) commented

2023-06-29 06:09:21 +00:00

#5406 is added

cyber-toad (Migrated from github.com) reviewed 2023-06-29 06:21:09 +00:00

kml/serdes_gpx.cpp

cyber-toad (Migrated from github.com) commented

2023-06-29 06:21:09 +00:00

Check for empty string added

cyber-toad (Migrated from github.com) reviewed 2023-06-29 06:21:30 +00:00

kml/serdes_gpx.cpp

					
				@ -140,6 +140,39 @@ void GpxParser::ParseColor(std::string const & value)

				  m_color = kml::ToRGBA(colorBytes[0], colorBytes[1], colorBytes[2], (char)255);

				}

cyber-toad (Migrated from github.com) commented

2023-06-29 06:21:30 +00:00

Fixed

biodranik (Migrated from github.com) approved these changes 2023-06-29 06:22:02 +00:00

biodranik (Migrated from github.com) left a comment

Thanks! We can later reuse the same test tracks to check if ele works.

biodranik (Migrated from github.com) approved these changes 2023-06-30 09:04:04 +00:00

kml/serdes_gpx.cpp

biodranik (Migrated from github.com) commented

2023-06-30 09:01:34 +00:00

Looks like using namespace std::string_view_literals; above is not used in the file.

Looks like `using namespace std::string_view_literals;` above is not used in the file.

kml/serdes_gpx.cpp

					
				@ -140,6 +140,39 @@ void GpxParser::ParseColor(std::string const & value)

				  m_color = kml::ToRGBA(colorBytes[0], colorBytes[1], colorBytes[2], (char)255);

				}

biodranik (Migrated from github.com) commented

2023-06-30 09:02:35 +00:00

nit: According to the code below, this initialization is unnecessary.

cyber-toad (Migrated from github.com) reviewed 2023-06-30 21:00:34 +00:00

kml/serdes_gpx.cpp

cyber-toad (Migrated from github.com) commented

2023-06-30 21:00:34 +00:00

Fixed

cyber-toad (Migrated from github.com) reviewed 2023-06-30 21:00:41 +00:00

kml/serdes_gpx.cpp

					
				@ -140,6 +140,39 @@ void GpxParser::ParseColor(std::string const & value)

				  m_color = kml::ToRGBA(colorBytes[0], colorBytes[1], colorBytes[2], (char)255);

				}

cyber-toad (Migrated from github.com) commented

2023-06-30 21:00:41 +00:00

Fixed

This repo is archived. You cannot comment on pull requests.

No reviewers

biodranik

No labels

Android Automotive (AAOS)

API

AppGallery

AppStore

Battery and Performance

External Map Datasets

F-Droid

Fonts

Frequently User Reported

No milestone

No project

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#5400

				`@ -140,6 +140,39 @@ void GpxParser::ParseColor(std::string const & value)`
				`m_color = kml::ToRGBA(colorBytes[0], colorBytes[1], colorBytes[2], (char)255);`

Rows
Columns