Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios] Add removeStyleImage to MGLMapView #14769

Merged
merged 3 commits into from
Jul 2, 2019
Merged

Conversation

fabian-guerra
Copy link
Contributor

Fixes #14731

@fabian-guerra fabian-guerra added the iOS Mapbox Maps SDK for iOS label May 24, 2019
@fabian-guerra fabian-guerra added this to the release-oolong milestone May 24, 2019
@fabian-guerra fabian-guerra self-assigned this May 24, 2019
@julianrex
Copy link
Contributor

We should wait for #14763 to land, as it moves a few things around.

@param mapView The map view that is evicting the image.
@param imageName The image name that is going to be removed.
*/
- (BOOL)mapView:(MGLMapView *)mapView removeStyleImage:(NSString *)imageName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- (BOOL)mapView:(MGLMapView *)mapView removeStyleImage:(NSString *)imageName;
- (BOOL)mapView:(MGLMapView *)mapView shouldRemoveStyleImage:(NSString *)imageName;

@fabian-guerra fabian-guerra force-pushed the fabian-eviction-14731 branch from 042f9b3 to 12cb77e Compare June 1, 2019 00:56
@fabian-guerra fabian-guerra marked this pull request as ready for review June 1, 2019 01:06
@fabian-guerra fabian-guerra requested a review from a team June 1, 2019 01:06
@fabian-guerra fabian-guerra added macOS Mapbox Maps SDK for macOS needs changelog Indicates PR needs a changelog entry prior to merging. labels Jun 1, 2019
@fabian-guerra fabian-guerra requested a review from julianrex June 10, 2019 18:06
Copy link
Contributor

@julianrex julianrex left a comment

Choose a reason for hiding this comment

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

Looks good - thank you! Let's think about what tests might be relevant here.

Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

Suggested changelog entry:

  • Added the -[MGLMapViewDelegate mapView:shouldRemoveStyleImage:] method for optimizing style image caching. (#14769)

goes over the cache size or when the image's requesting tile is destroyed.

@param mapView The map view that is evicting the image.
@param imageName The image name that is going to be removed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Document the return value as well. (×2)

Asks the delegate whether the map view should evict cached images.

This method is called in two scenarios: when the cumulative size of unused images
goes over the cache size or when the image's requesting tile is destroyed.
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is called in two scenarios: when the cumulative size of unused images exceeds the cache size or when the last tile that includes the image is removed from memory.

(×2)

@@ -333,6 +333,17 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (nullable NSViewController *)mapView:(MGLMapView *)mapView calloutViewControllerForAnnotation:(id <MGLAnnotation>)annotation;

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

This method appears under the “Managing Callout Popovers” heading in the header and in documentation:

#pragma mark Managing Callout Popovers

Create a new heading for “Managing the Style” or move this method up to the “Loading the Map” section.

@@ -277,6 +277,17 @@ NS_ASSUME_NONNULL_BEGIN

- (nullable UIImage *)mapView:(MGLMapView *)mapView didFailToLoadImage:(NSString *)imageName;

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

This method appears under the “Managing Callout Views” heading in the header and in documentation:

#pragma mark Managing Callout Views

Create a new heading for “Managing the Style” or move this method up to the “Loading the Map” section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On iOS is under the right header.

@fabian-guerra fabian-guerra force-pushed the fabian-eviction-14731 branch from 12cb77e to 0eda0b9 Compare July 1, 2019 23:00
@fabian-guerra fabian-guerra removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Jul 1, 2019
@fabian-guerra fabian-guerra force-pushed the fabian-eviction-14731 branch from 0eda0b9 to c24d67b Compare July 2, 2019 18:34
@fabian-guerra fabian-guerra merged commit 12e0a6b into master Jul 2, 2019
@fabian-guerra fabian-guerra deleted the fabian-eviction-14731 branch July 2, 2019 23:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ios, macos] Eviction of cached images bindings for darwin
4 participants