Fix #1178 background image scaling in gpsgraphic filter#1179
Fix #1178 background image scaling in gpsgraphic filter#1179ddennedy merged 3 commits intomltframework:masterfrom
Conversation
tranform latitude for gpsgraphic, as the tile server usually use a different coordinate represenation EPSG 3857, which leads to misscaling without transformation, especially on large areas. Transformation is done during loading of the file and interpolation, so hopefully not affecting speed of filter
There was a problem hiding this comment.
Pull request overview
This PR fixes background image scaling issues in the gpsgraphic filter by transforming latitude coordinates from GPS format (EPSG:4326/WGS84) to the tile server's coordinate system (EPSG:3857). The transformation prevents miscaling, particularly noticeable over large areas, by applying projection during file loading and interpolation.
Key changes:
- Added
lat_projectedfield to GPS data structures to store EPSG:3857 projected latitude values - Implemented
project_latitude()function to perform coordinate transformation - Updated aspect ratio calculation to use projected coordinates instead of haversine distance calculations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/modules/qt/gps_parser.h | Added lat_projected field to gps_point_raw and gps_point_proc structs and their initialization constants |
| src/modules/qt/gps_parser.cpp | Implemented latitude projection function and populated lat_projected during parsing and interpolation |
| src/modules/qt/filter_gpstext.cpp | Updated to copy lat_projected field when using raw GPS points |
| src/modules/qt/filter_gpsgraphic.cpp | Modified to use projected latitude for display and simplified aspect ratio calculation using coordinate differences |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| gps_points_p[i].lon = lon_sum / nr_div; | ||
| } else { | ||
| gps_points_p[i].lat = gps_points_r[i].lat; | ||
| gps_points_p[i].lat_projected = gps_points_r[i].lat; |
There was a problem hiding this comment.
Should be projecting the latitude value using project_latitude(gps_points_r[i].lat) instead of directly assigning the unprojected latitude. This inconsistency will cause incorrect rendering when smoothing is disabled and no interpolation occurs.
| gps_points_p[i].lat_projected = gps_points_r[i].lat; | |
| gps_points_p[i].lat_projected = project_latitude(gps_points_r[i].lat); |
There was a problem hiding this comment.
I am willing to merge this if you address some of these review comments. Why haven’t you?
There was a problem hiding this comment.
Should be fixed with the now commit now
There was a problem hiding this comment.
I think you missed this one on the else block, the assignment on line 1036 should either be:
gps_points_p[i].lat_projected = gps_points_r[i].lat_projected; //because it was computed at file read and is the original raw point
or
gps_points_p[i].lat_projected = project_latitude(gps_points_r[i].lat); //to just compute it again
|
@ddennedy is there a .zip created with the built app from this PR to do some hands-on tests? I'm curious how big the difference is between the 2 systems as I didn't notice any major shifts in my personal tracks. I had a first look at the changes and they seem to cover well most of the cases I can think of. I'll have another look the next few days as I'm short on time today. There's 3 places where I think we need to print the original unprojected lat point:
The get_src change makes getting the original lat point rather messy, so maybe it's faster to just make a function to compute the reverse of project_latitude? For the find_minmax_of_data() change, I think it would be better to just do the conversion at the very end, not inside the for, to avoid unnecessary computations. This would also quickly fix the 3rd bullet point by moving the middle_point computation before the conversion. (I should have added an exact line comment for this one but it's 1AM and need to go). |
|
The error depends on the distance I'ld say, as it's not a linear scaling. Thats why it doesn't help to simply scale the image itself. I guess you might be right with min/max, maybe the projection should only be done there when the aspect ratio is calculated, so at other places the unprojected value can be used. In case of speed it should not matter, as I just replaced lat with projected lat there, so no additional comparison / calculation. |
Thank you for the example, I haven't used such long tracks on detailed maps before so I didn't catch the problem in my usual exports. This is a good fix. Just address the small comments (including the ones from copilot) and it's ok from me. |
- Fixed now dot text - Fixed grid text - Removed unused var - method to calculate back to gps coordinates from projected coordinate
tranform latitude for gpsgraphic, as the tile server usually use a different coordinate represenation EPSG 3857, which leads to misscaling without transformation, especially on large areas. Transformation is done during loading of the file and interpolation, so hopefully not affecting speed of filter