Repository navigation
GPS graphic: optimize drawing by skipping overlapping lines - #1338
Merged
Merged
Conversation
… updates_per_second to -1
… move a pixel also optimized index iteration on the drawing loop [rebased from branch]
Contributor
Author
|
Quick question: should I ask for a copilot review every pr? I don't know if this is free/paid/limited free credits and if lite/balanced matters. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
GPS drawing has unresolved correctness and logging issues, along with a copyright-year update.
1 open finding
What changed in this PR
This pull request optimizes GPS rendering and timestamp processing for faster exports.
Changes:
- Skips redundant GPS segments and jumps between valid points.
- Calculates frame time directly from frame position and FPS.
- Caches GPS processing timestamps and updates parser interfaces.
| File | Summary |
|---|---|
src/tests/test_mod_qt_gps/test_mod_qt_gps.cpp |
Updated test declarations; copyright year should be updated to 2026. |
src/modules/qt/gps_parser.h |
Updated parser declaration. |
src/modules/qt/gps_parser.cpp |
Updated parser definition and format parameter. |
src/modules/qt/gps_drawing.cpp |
Optimized track drawing; repeated logging, dots-only rendering, and data-dependent color changes need correction. |
src/modules/qt/filter_gpstext.cpp |
Added processing timestamp caching. |
src/modules/qt/filter_gpsgraphic.h |
Renamed the valid-point helper. |
src/modules/qt/filter_gpsgraphic.cpp |
Updated frame-time calculation and valid-point handling. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or 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
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.
The GPS graphic change is the major one, obviously depends on the track length and zoom level but a best case scenario where fully drawing an 8h GPS track on a 1 min color clip renders in
36snow vs1m15sbefore (with the default x264 export).Point skipping is huge, but there's still plenty of overhead so only 2x faster export:
The mutex change drops the frame time calculation from ~1ms to 0.05ms (from memory, I had these written down but I overwrote them by mistake). The last_read_gps_processing_start_time also drops from a ~1ms to under 0.1ms.
But overall the GPS text filter is still slow, not sure why, the entire filter_process takes ~1ms, almost identical to GPS drawing but text is significantly slower to export. It's still slightly faster than before so oh well...
(I think some commits from the old PR show up because I had merge conflicts on git rebase master)