Context Menu optimization - open POI context menu without blocking the main thread#5536
Conversation
|
|
||
| private func resolveDetailedObjectInBackground() { | ||
| guard let renderedObject else { return } | ||
| DispatchQueue.global(qos: .userInitiated).async { [weak self] in |
There was a problem hiding this comment.
This can introduce a race condition. An outdated background request may complete later and overwrite the current state. Consider validating that the result is still relevant before applying it
| if (!strongSelf) | ||
| return; | ||
|
|
||
| strongSelf->_nearestWiki = [results copy]; |
There was a problem hiding this comment.
This changes the nearest wiki lookup path from processNearestWiki to fetchNearestPoi, but the old wiki language filtering seems to be lost!
| } | ||
|
|
||
|
|
||
| private func resolveDetailedObjectInBackground() { |
There was a problem hiding this comment.
Async detail lookup can update the wrong target point PlaceDetailsViewController.resolveDetailedObjectInBackground / updateTargetPoint(with:) The background lookup resolves later, then blindly calls mapPanel.getCurrentTargetPoint() and mutates it. If the user taps another map object before the lookup finishes, the old controller can update the new current target’s icon and refresh the map with stale data
| { | ||
| controller = [[RenderedObjectViewController alloc] initWithRenderedObject:targetPoint.targetObj]; | ||
| } | ||
| controller = [[PlaceDetailsViewController alloc] initWithRenderedObject:targetPoint.targetObj]; |
There was a problem hiding this comment.
This is more than just moving searchDetailedObject off the main thread: it removes the old fallback to RenderedObjectViewController for rendered objects where no BaseDetailsObject is found.
Previously:
- details found ->
PlaceDetailsViewController - details not found ->
RenderedObjectViewController
Now every rendered object opens PlaceDetailsViewController(initWithRenderedObject:), and if the async lookup returns nil it remains there. Please confirm this new rendered-only state is fully equivalent to the old RenderedObjectViewController behavior, including name/type/icon/OSM URL, target title updates, and rows shown when details are unavailable.
Also, the async lookup result should only be applied if the current target still corresponds to the same rendered object; otherwise a delayed lookup can update the wrong target after the user taps another object.
There was a problem hiding this comment.
?
case OATargetPOI:
{
controller = [[OAPOIViewController alloc] initWithPOI:targetPoint.targetObj];
if (selectedObject && [selectedObject isKindOfClass:BaseDetailsObject.class])
{
BaseDetailsObject *detailsObject = [OAAmenitySearcher.sharedInstance searchDetailedObject:selectedObject];
if (detailsObject)
{
controller = [[PlaceDetailsViewController alloc] initWithPoi:targetPoint.targetObj detailsObject:detailsObject renderedObject:targetPoint.targetObj];
}
else
{
controller = [[RenderedObjectViewController alloc] initWithRenderedObject:targetPoint.targetObj];
}
}
break;
}
| guard let mapPanel = OARootViewController.instance()?.mapPanel, | ||
| let targetPoint = mapPanel.getCurrentTargetPoint() else { return } | ||
|
|
||
| targetPoint.title = amenity.nameLocalized ?? amenity.name |
|
|
||
| override func buildDescription(_ rows: NSMutableArray) { | ||
| if detailsObject == nil { | ||
| super.buildDescription(rows) |
| { | ||
| controller = [[RenderedObjectViewController alloc] initWithRenderedObject:targetPoint.targetObj]; | ||
| } | ||
| controller = [[PlaceDetailsViewController alloc] initWithRenderedObject:targetPoint.targetObj]; |
There was a problem hiding this comment.
?
case OATargetPOI:
{
controller = [[OAPOIViewController alloc] initWithPOI:targetPoint.targetObj];
if (selectedObject && [selectedObject isKindOfClass:BaseDetailsObject.class])
{
BaseDetailsObject *detailsObject = [OAAmenitySearcher.sharedInstance searchDetailedObject:selectedObject];
if (detailsObject)
{
controller = [[PlaceDetailsViewController alloc] initWithPoi:targetPoint.targetObj detailsObject:detailsObject renderedObject:targetPoint.targetObj];
}
else
{
controller = [[RenderedObjectViewController alloc] initWithRenderedObject:targetPoint.targetObj];
}
}
break;
}
|
|
||
| while (osmwiki.count < kNearbyPoiMaxCount && radius <= kNearbyPoiMaxRadius) | ||
| { | ||
| osmwiki = [[OAAmenitySearcher findPOIsByTagName:nil name:nil location:locI categoryName:OSM_WIKI_CATEGORY poiTypeName:nil radius:radius] mutableCopy]; |
There was a problem hiding this comment.
Wiki targets lose “Wikipedia around” results
| additionalInfoKeys: additionalInfoKeys | ||
| ) | ||
| } | ||
| let byType = resolvedTypeName(amenity) |
There was a problem hiding this comment.
Type label logic no longer matches Android and can become less specific
There was a problem hiding this comment.
typeStr = searchObjectTypeByAmenityTags(that.getAmenity());
if (Algorithms.isEmpty(typeStr)) {
typeStr = searchObjectNameByIconRes();
}
if (Algorithms.isEmpty(typeStr)) {
typeStr = searchObjectNameByRawTags(...);
}
return typeStr != null ? typeStr : super.getTypeStr();
| }() | ||
|
|
||
| let byType = resolvedTypeName(amenity) | ||
| let byTags = amenity.flatMap { searchObjectTypeByAmenityTags($0) } |
There was a problem hiding this comment.
This computes all fallback candidates eagerly, even though type resolution should short-circuit. Please keep the Android-like fallback order and return as soon as a non-empty type string is found, so we avoid unnecessary tag scans/translations and preserve priority semantics
| return cachedNameStr ?? "" | ||
| } | ||
|
|
||
| func actualContentFromIconRes() -> String? { |
| return content | ||
| } | ||
|
|
||
| func searchObjectNameByIconRes() -> String? { |
Fix: resolve the detailed object in the background. The sheet is now presented
immediately from the tapped rendered object (name/type/icon already known) and
is enriched via
rebuildRowsonce the lookup completes — matching Android,which loads this data off the main thread.
Result on the test device: perceived open ~1.6s → ~0.86s, no main-thread freeze.
Note:
searchDetailedObjectnow runs off the main thread.