Skip to content

Context Menu optimization - open POI context menu without blocking the main thread#5536

Merged
alex-dev-neo merged 11 commits into
masterfrom
task_5337_context_menu_slow_version_3
Jul 22, 2026
Merged

Context Menu optimization - open POI context menu without blocking the main thread#5536
alex-dev-neo merged 11 commits into
masterfrom
task_5337_context_menu_slow_version_3

Conversation

@aleksandr-tata

Copy link
Copy Markdown
Contributor

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 rebuildRows once 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: searchDetailedObject now runs off the main thread.

@aleksandr-tata
aleksandr-tata requested a review from tigrim July 14, 2026 14:35
@aleksandr-tata aleksandr-tata linked an issue Jul 14, 2026 that may be closed by this pull request
1 task

private func resolveDetailedObjectInBackground() {
guard let renderedObject else { return }
DispatchQueue.global(qos: .userInitiated).async { [weak self] in

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.

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

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

if (!strongSelf)
return;

strongSelf->_nearestWiki = [results copy];

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.

This changes the nearest wiki lookup path from processNearestWiki to fetchNearestPoi, but the old wiki language filtering seems to be lost!

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

}


private func resolveDetailedObjectInBackground() {

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.

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

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

{
controller = [[RenderedObjectViewController alloc] initWithRenderedObject:targetPoint.targetObj];
}
controller = [[PlaceDetailsViewController alloc] initWithRenderedObject:targetPoint.targetObj];

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.

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.

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

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.

?

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

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.

?


override func buildDescription(_ rows: NSMutableArray) {
if detailsObject == nil {
super.buildDescription(rows)

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.

?

{
controller = [[RenderedObjectViewController alloc] initWithRenderedObject:targetPoint.targetObj];
}
controller = [[PlaceDetailsViewController alloc] initWithRenderedObject:targetPoint.targetObj];

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.

?

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];

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.

Wiki targets lose “Wikipedia around” results

additionalInfoKeys: additionalInfoKeys
)
}
let byType = resolvedTypeName(amenity)

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.

Type label logic no longer matches Android and can become less specific

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.

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) }

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.

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? {

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.

private?

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

return content
}

func searchObjectNameByIconRes() -> String? {

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.

private?

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

@alex-dev-neo
alex-dev-neo merged commit e00ab2c into master Jul 22, 2026
@alex-dev-neo
alex-dev-neo deleted the task_5337_context_menu_slow_version_3 branch July 22, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Context menu opens twice as slowly on iOS compared to Android

3 participants