Skip to content

Sync chart cursor across route analysis charts #5680

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.

Sign up for GitHub

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

Open
aleksandr-tata wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Sync chart cursor across route analysis charts #5680

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
54503b9
Sync chart cursor across route analysis charts
aleksandr-tata Aug 21, 2026
dfea27a
Fix cursor scrubbing on route attribute charts
aleksandr-tata Aug 21, 2026
b3ec09d
Fix route chart synchronization after zoom and cell reuse
aleksandr-tata Aug 24, 2026
896d765
Fix route chart cursor alignment and distance scale labels
aleksandr-tata Aug 24, 2026
95340fc
Fix route chart cursor snapping and highlight reset
aleksandr-tata Aug 24, 2026
0b1bbae
Remove redundant weak captures from chart updates
aleksandr-tata Aug 25, 2026
0923f9c
Remove redundant Objective-C annotations
aleksandr-tata Aug 25, 2026
979be64
Refactor chart registration after layout
aleksandr-tata Aug 25, 2026
2357bfc
Align route bar charts with primary distance range
aleksandr-tata Aug 25, 2026
eb7572a
Merge branch 'master' into task_5669_sync_chart_cursor
aleksandr-tata Aug 26, 2026
57f70dc
Fix chart axis ranges after switching X-axis mode
aleksandr-tata Aug 26, 2026
2abd298
Fix map marker update during chart zoom and pan
aleksandr-tata Aug 27, 2026
54b7d61
Unregister off-screen route bar charts
aleksandr-tata Aug 27, 2026
be13ac0
Preserve chart Y transform during viewport sync
aleksandr-tata Aug 27, 2026
a7995a3
Fix chart state handling when primary chart is off-screen
aleksandr-tata Aug 28, 2026
02add7c
Avoid duplicate chart viewport synchronization
aleksandr-tata Aug 28, 2026
82079a0
Fix chart viewport and selection state synchronization
aleksandr-tata Sep 2, 2026
789dfdf
Track registered charts across cell reuse
aleksandr-tata Sep 2, 2026
e2ae519
Restore bar chart viewports after axis changes
aleksandr-tata Sep 2, 2026
a8a2bd5
Filter chart selection reset events
aleksandr-tata Sep 2, 2026
0485f92
Avoid duplicate viewport sync in Route Details
aleksandr-tata Sep 2, 2026
b71d286
Use generated Objective-C chart sync selector
aleksandr-tata Sep 2, 2026
ce941b3
Fix chart distance basis for routes with gaps
aleksandr-tata Sep 3, 2026
14699f6
Merge branch 'master' into task_5669_sync_chart_cursor
aleksandr-tata Sep 7, 2026
3f4804c
Fix chart registration across cell reuse
aleksandr-tata Sep 7, 2026
89cd1eb
Fix route chart alignment across GPX gaps
aleksandr-tata Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions OsmAnd.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,7 @@
46903D6B29B5F77A004459F1 /* OABaseWebViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 46903D6A29B5F77A004459F1 /* OABaseWebViewController.xib */; };
46903D6E29B664B2004459F1 /* OAWikiLanguagesWebViewContoller.m in Sources */ = {isa = PBXBuildFile; fileRef = 46903D6D29B664B2004459F1 /* OAWikiLanguagesWebViewContoller.m */; };
4695B25F2CF0A5D3003FB38A /* TrackChartHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4695B25E2CF0A5CB003FB38A /* TrackChartHelper.swift */; };
A5C7E00133F0000100C0DE01 /* RouteChartSynchronizer.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5C7E00033F0000100C0DE01 /* RouteChartSynchronizer.swift */; };
46993C0A2986BCB70047F2F5 /* img_help_announcement_time_day@3x.png in Resources */ = {isa = PBXBuildFile; fileRef = 46993C092986BCB60047F2F5 /* img_help_announcement_time_day@3x.png */; };
469F0C222C627E2A00874CC6 /* PaletteCollectionViewCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 469F0C212C627E2A00874CC6 /* PaletteCollectionViewCell.swift */; };
469F0C242C627F9000874CC6 /* PaletteCollectionViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 469F0C232C627F9000874CC6 /* PaletteCollectionViewCell.xib */; };
Expand Down Expand Up @@ -4679,6 +4680,7 @@
46903D6C29B6649A004459F1 /* OAWikiLanguagesWebViewContoller.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = OAWikiLanguagesWebViewContoller.h; sourceTree = "<group>"; };
46903D6D29B664B2004459F1 /* OAWikiLanguagesWebViewContoller.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = OAWikiLanguagesWebViewContoller.m; sourceTree = "<group>"; };
4695B25E2CF0A5CB003FB38A /* TrackChartHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TrackChartHelper.swift; sourceTree = "<group>"; };
A5C7E00033F0000100C0DE01 /* RouteChartSynchronizer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RouteChartSynchronizer.swift; sourceTree = "<group>"; };
46993C092986BCB60047F2F5 /* img_help_announcement_time_day@3x.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; name = "img_help_announcement_time_day@3x.png"; path = "Resources/Icons/img_help_announcement_time_day@3x.png"; sourceTree = "<group>"; };
469F0C212C627E2A00874CC6 /* PaletteCollectionViewCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaletteCollectionViewCell.swift; sourceTree = "<group>"; };
469F0C232C627F9000874CC6 /* PaletteCollectionViewCell.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = PaletteCollectionViewCell.xib; sourceTree = "<group>"; };
Expand Down Expand Up @@ -12409,6 +12411,7 @@
3284BEBB2EABA32C0011A9BA /* GpxAppearanceInfo.swift */,
FA8516EF2D42946D00B321C6 /* GpxDataItemHandler.swift */,
4695B25E2CF0A5CB003FB38A /* TrackChartHelper.swift */,
A5C7E00033F0000100C0DE01 /* RouteChartSynchronizer.swift */,
DA5A7AC026C563A100F274C7 /* OAGPXDatabase.h */,
DA5A7ABB26C563A100F274C7 /* OAGPXDatabase.mm */,
320F71352A8643550071C0E7 /* OAGPXDocumentAdapter.h */,
Expand Down Expand Up @@ -18324,6 +18327,7 @@
DA9F841C29CC96EF00127DAE /* OAShapeLayer.m in Sources */,
46CBC9722B96126C005CC244 /* GlideBaseWidget.swift in Sources */,
4695B25F2CF0A5D3003FB38A /* TrackChartHelper.swift in Sources */,
A5C7E00133F0000100C0DE01 /* RouteChartSynchronizer.swift in Sources */,
DA5A835526C563A800F274C7 /* OAGPXTrackCell.m in Sources */,
27E3C2032DF1ADD40053958E /* OAStreetNameWidgetParams.mm in Sources */,
DA5A844926C563A900F274C7 /* OADiscountToolbarViewController.mm in Sources */,
Expand Down
157 changes: 82 additions & 75 deletions Sources/Controllers/PlanRoute/Tabs/PlanRouteAnalyzeViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,25 @@ private struct AnalyzeStatItem {
}

private final class AnalyzeChartDelegateProxy: NSObject, ChartViewDelegate {
var onNothingSelected: (() -> Void)?
var onValueSelected: (() -> Void)?
var onTranslated: (() -> Void)?
var onNothingSelected: ((ChartViewBase) -> Void)?
var onValueSelected: ((ChartViewBase, Highlight) -> Void)?
var onTranslated: ((ChartViewBase) -> Void)?
var onScaled: ((ChartViewBase) -> Void)?

func chartValueNothingSelected(_ chartView: ChartViewBase) {
onNothingSelected?()
onNothingSelected?(chartView)
}

func chartValueSelected(_ chartView: ChartViewBase, entry: ChartDataEntry, highlight: Highlight) {
onValueSelected?()
onValueSelected?(chartView, highlight)
}

func chartTranslated(_ chartView: ChartViewBase, dX: CGFloat, dY: CGFloat) {
onTranslated?()
onTranslated?(chartView)
}

func chartScaled(_ chartView: ChartViewBase, scaleX: CGFloat, scaleY: CGFloat) {
onScaled?(chartView)
}
}

Expand All @@ -62,6 +67,7 @@ final class PlanRouteAnalyzeViewController: UIViewController, PlanRouteTabConten
var onAttachToRoadsRequested: (() -> Void)?

private let tableView = CancelableTableView(frame: .zero, style: .plain)
private let chartSynchronizer = RouteChartSynchronizer()

private var selectedYAxisTypes: [NSNumber] = [
NSNumber(value: GPXDataSetType.altitude.rawValue),
Expand All @@ -81,27 +87,32 @@ final class PlanRouteAnalyzeViewController: UIViewController, PlanRouteTabConten
private var cachedSyntheticSteepnessSignature: Double = -1
private var pendingSteepnessSignature: Double = -1
private var lastRenderState: AnalyzeRenderState?
private var currentChartDataSignature: String?
private var trackChartFilePath: String?
private var trackChartHelper: TrackChartHelper?
private var highlightDrawX: CGFloat = -1
private var lastTranslation: CGPoint = .zero
private weak var dataSource: PlanRouteAnalyzeDataSource?
private weak var chartView: ElevationChart?
private weak var yAxisButton: UIButton?
private weak var xAxisButton: UIButton?
private var chartView: ElevationChart?

@tigrim tigrim Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a strong reference really needed here? It keeps the chart alive after its cell goes off-screen, while the synchronizer itself uses weak chart references

private lazy var chartDelegateProxy: AnalyzeChartDelegateProxy = {
let proxy = AnalyzeChartDelegateProxy()
proxy.onNothingSelected = { [weak self] in
proxy.onNothingSelected = { [weak self] _ in

@alex-dev-neo alex-dev-neo Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onValueSelected below filters on chart === chartView, but this one clears the whole synchronized selection for any chart. The bar charts share this proxy and run with highlightPerDragEnabled = true, so a DGCharts drag on a bar chart that resolves to a nil highlight would wipe the cursor everywhere.

It looks unlikely to fire in practice given maxHighlightDistance = 10_000 and a single bar entry, but the same guard here would make it symmetric.

self?.chartSynchronizer.clearSynchronizedHighlights()
self?.hideChartLocation()
}
proxy.onValueSelected = { [weak self] in
self?.refreshChartOnMap()
proxy.onValueSelected = { [weak self] chart, highlight in
guard let self, let chartView, chart === chartView else { return }
chartSynchronizer.syncHighlight(highlight, sourceChart: chartView)
refreshChartOnMap()
}
proxy.onTranslated = { [weak self] chart in
self?.handleChartViewPortChanged(chart)
}
proxy.onTranslated = { [weak self] in
self?.handleChartTranslated()
proxy.onScaled = { [weak self] chart in
self?.handleChartViewPortChanged(chart)
}
return proxy
}()
private weak var dataSource: PlanRouteAnalyzeDataSource?
private weak var yAxisButton: UIButton?
private weak var xAxisButton: UIButton?

private var currentState: AnalyzeState {
cachedState
Expand Down Expand Up @@ -160,6 +171,13 @@ final class PlanRouteAnalyzeViewController: UIViewController, PlanRouteTabConten
}
wasCalculatingElevation = isElevationCalculating
let analysisData = dataSource?.analysisData
let nextChartDataSignature = chartDataSignature(for: analysisData)
if currentChartDataSignature != nextChartDataSignature {
chartSynchronizer.reset()
chartView = nil
currentChartDataSignature = nextChartDataSignature
hideChartLocation()
}
cachedHasElevationData = analysisData?.hasElevationData == true
cachedHasSpeedData = analysisData?.hasSpeedData == true
cachedHasOverviewData = cachedHasElevationData || cachedHasSpeedData
Expand All @@ -172,6 +190,8 @@ final class PlanRouteAnalyzeViewController: UIViewController, PlanRouteTabConten
roadAttributeStatistics: cachedRoadAttributeStatistics)
let renderState = makeRenderState(analysisData: analysisData)
if renderState.graphSignature == nil {
chartSynchronizer.reset()
chartView = nil
hideChartLocation()
}
applyRenderState(renderState)
Expand Down Expand Up @@ -244,6 +264,7 @@ final class PlanRouteAnalyzeViewController: UIViewController, PlanRouteTabConten
secondType: secondType,
axisType: selectedXAxisType,
calcWithoutGaps: GpxUtils.calcWithoutGaps(gpxFile, gpxDataItem: gpxItem, overrideIsGeneralTrack: true))
chartSynchronizer.setPrimaryChart(chart)
if !chart.highlighted.isEmpty {
refreshChartOnMap()
}
Expand All @@ -266,12 +287,9 @@ final class PlanRouteAnalyzeViewController: UIViewController, PlanRouteTabConten
segment: segment)
}

private func bindChartGestures(_ chart: ElevationChart) {
private func bindChartGestures(_ chart: BarLineChartViewBase) {
chart.delegate = chartDelegateProxy
chart.gestureRecognizers?.forEach { recognizer in
if recognizer is UIPanGestureRecognizer {
recognizer.addTarget(self, action: #selector(onChartScrolled(_:)))
}
recognizer.addTarget(self, action: #selector(onChartGesture(_:)))
}
}
Expand Down Expand Up @@ -306,57 +324,9 @@ final class PlanRouteAnalyzeViewController: UIViewController, PlanRouteTabConten
dataSource?.hideChartHighlight()
}

@objc private func onChartScrolled(_ recognizer: UIPanGestureRecognizer) {
guard let chart = recognizer.view as? ElevationChart else { return }

if recognizer.state == .changed {
if chart.lowestVisibleX > 0.1,
roundedChartValue(chart.highestVisibleX) != roundedChartValue(chart.chartXMax) {
lastTranslation = recognizer.translation(in: chart)
return
}

let touchPoint = recognizer.location(in: chart)
let translation = recognizer.translation(in: chart)
let highlightX = chart.isFullyZoomedOut
? touchPoint.x
: highlightDrawX + (lastTranslation.x - translation.x)
guard let highlight = chart.getHighlightByTouchPoint(CGPoint(x: highlightX, y: 0)) else { return }
chart.lastHighlighted = highlight
chart.highlightValue(highlight, callDelegate: true)
} else if recognizer.state == .ended {
lastTranslation = .zero
if let highlight = chart.highlighted.first {
highlightDrawX = highlight.drawX
}
}
}

@objc private func onChartGesture(_ recognizer: UIGestureRecognizer) {
guard let chart = recognizer.view as? ElevationChart else { return }

if recognizer.state == .began {
if let highlight = chart.highlighted.first {
highlightDrawX = highlight.drawX
} else {
highlightDrawX = -1
}
} else if (recognizer is UIPinchGestureRecognizer
|| (recognizer is UITapGestureRecognizer
&& (recognizer as? UITapGestureRecognizer)?.numberOfTapsRequired == 2))
&& recognizer.state == .ended {
refreshChartOnMap()
}
}

private func roundedChartValue(_ value: Double) -> Double {
(value * 10).rounded() / 10
}

private func handleChartTranslated() {
guard let chart = self.chartView, highlightDrawX != -1 else { return }
guard let highlight = chart.getHighlightByTouchPoint(CGPoint(x: highlightDrawX, y: 0)) else { return }
chart.highlightValue(highlight, callDelegate: true)
private func handleChartViewPortChanged(_ chart: ChartViewBase) {
guard let chart = chart as? BarLineChartViewBase else { return }
chartSynchronizer.syncViewPort(from: chart)
}

private func resolvedYAxisTypes() -> (GPXDataSetType, GPXDataSetType) {
Expand Down Expand Up @@ -439,6 +409,17 @@ final class PlanRouteAnalyzeViewController: UIViewController, PlanRouteTabConten
].joined(separator: "|")
}

private func chartDataSignature(for analysisData: PlanRouteAnalysisData?) -> String? {
guard let analysis = analysisData?.gpxAnalysis,
let gpxFile = analysisData?.gpxFile else { return nil }
return [
gpxFile.path,
String(analysis.totalDistance),
String(analysis.timeSpan),
String(analysis.startTime)
].joined(separator: "|")
}

private func statsSignature(for analysisData: PlanRouteAnalysisData?) -> String? {
guard cachedHasOverviewData, let analysisData else { return nil }
return [
Expand Down Expand Up @@ -635,8 +616,6 @@ extension PlanRouteAnalyzeViewController: UITableViewDataSource {

let chart = ElevationChart(frame: .zero)
chart.translatesAutoresizingMaskIntoConstraints = false
chartView = chart
bindChartGestures(chart)

let gpxItem = dataItem(for: gpxFile)
let useHours = (analysis.timeSpan / Self.millisecondsPerHour) > 0
Expand All @@ -655,6 +634,9 @@ extension PlanRouteAnalyzeViewController: UITableViewDataSource {
axisType: selectedXAxisType,
calcWithoutGaps: GpxUtils.calcWithoutGaps(gpxFile, gpxDataItem: gpxItem, overrideIsGeneralTrack: true))
chart.dragYEnabled = false
chartView = chart
chartSynchronizer.setPrimaryChart(chart)
bindChartGestures(chart)

let recalcSeparator = SeparatorView()
recalcSeparator.translatesAutoresizingMaskIntoConstraints = false
Expand Down Expand Up @@ -871,7 +853,6 @@ extension PlanRouteAnalyzeViewController: UITableViewDataSource {
let isExpanded = expandedStatIndexes.contains(statIndex)

let barChart = HorizontalBarChartView(frame: .zero)
barChart.isUserInteractionEnabled = false
barChart.translatesAutoresizingMaskIntoConstraints = false
GpxUIHelper.refreshBarChart(chartView: barChart,
statistics: stat,
Expand All @@ -888,6 +869,8 @@ extension PlanRouteAnalyzeViewController: UITableViewDataSource {
rightAxis.drawGridLinesEnabled = true
rightAxis.gridColor = .chartAxisGridLine
rightAxis.labelTextColor = .textColorSecondary
chartSynchronizer.registerBarChart(barChart)
bindChartGestures(barChart)

let legendView = isExpanded ? makeExpandedRoadAttrLegend(stat: stat) : makeCompactRoadAttrLegend(stat: stat)
card.addSubview(barChart)
Expand Down Expand Up @@ -1214,6 +1197,17 @@ extension PlanRouteAnalyzeViewController: UITableViewDataSource {

extension PlanRouteAnalyzeViewController: UITableViewDelegate {

func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) {
guard let cell = cell as? AnalyzeCardCell else { return }
if let chart = cell.cardView.subviews.first(where: { $0 is ElevationChart }) as? ElevationChart {
cell.layoutIfNeeded()

@tigrim tigrim Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@aleksandr-tata aleksandr-tata Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

chartSynchronizer.setPrimaryChart(chart)
} else if let barChart = cell.cardView.subviews.first(where: { $0 is HorizontalBarChartView }) as? HorizontalBarChartView {
cell.layoutIfNeeded()
chartSynchronizer.registerBarChart(barChart)

@tigrim tigrim Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we unregister the chart in didEndDisplaying? RouteChartSynchronizer keeps every registered bar chart in the weak hash table, so an off-screen/reused chart can still participate in viewport/highlight synchronization while it remains alive. It would be safer to explicitly unregister charts when their cells leave the screen

@aleksandr-tata aleksandr-tata Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

}
}

func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
tableView.deselectRow(at: indexPath, animated: false)
switch currentState {
Expand Down Expand Up @@ -1780,6 +1774,19 @@ private extension PlanRouteAnalyzeViewController {

private extension PlanRouteAnalyzeViewController {

@objc private func onChartGesture(_ recognizer: UIGestureRecognizer) {
let isDoubleTap = (recognizer as? UITapGestureRecognizer)?.numberOfTapsRequired == 2
guard recognizer is UIPinchGestureRecognizer || isDoubleTap,
recognizer.state == .ended,
let chart = recognizer.view as? BarLineChartViewBase else { return }
refreshChartOnMap()
DispatchQueue.main.async { [weak self, weak chart] in

@tigrim tigrim Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need [weak self, weak chart] here? There is no retain cycle with DispatchQueue.main.async

@aleksandr-tata aleksandr-tata Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

guard let self, let chart else { return }
chart.layoutIfNeeded()
self.chartSynchronizer.syncViewPort(from: chart)

@tigrim tigrim Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syncViewPort is already called from chartScaled / chartTranslated. Do we really need to call it again here after the gesture ends? This causes the same viewport/selection synchronization to run twice for pinch gestures and may also trigger an extra highlightValue(..., callDelegate: true) cycle

}
}

@objc private func onRecalculateTapped() {
showGetElevationSheet()
}
Expand Down
Loading