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

[ios] Change MGLAnnotationView.scalesWithViewingDistance default value to NO #11636

Merged
merged 1 commit into from
Apr 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ The 4.0._x_ series of releases will be the last to support iOS 8. The minimum iO

### Annotations

* Changed the default value of `MGLAnnotationView.scalesWithViewingDistance` to `NO`, to improve performance. If your use case involves many annotation views, consider keeping this property disabled. ([#11636](https://github.com/mapbox/mapbox-gl-native/pull/11636))
* Fixed an issue preventing `MGLAnnotationImage.image` from being updated. ([#10372](https://github.com/mapbox/mapbox-gl-native/pull/10372))
* Improved performance of `MGLAnnotationView`-backed annotations that have `scalesWithViewingDistance` enabled. ([#10951](https://github.com/mapbox/mapbox-gl-native/pull/10951))
* Fixed an issue where tapping a group of annotations may not have selected the nearest annotation. ([#11438](https://github.com/mapbox/mapbox-gl-native/pull/11438))
Expand Down
6 changes: 0 additions & 6 deletions platform/ios/app/MBXViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -1810,12 +1810,6 @@ - (MGLAnnotationView *)mapView:(MGLMapView *)mapView viewForAnnotation:(id<MGLAn
// Comment out the pin dropping functionality in the handleLongPress:
// method in this class to make draggable annotation views play nice.
annotationView.draggable = YES;

// Uncomment to force annotation view to maintain a constant size when
// the map is tilted. By default, annotation views will shrink and grow
// as they move towards and away from the horizon. Relatedly, annotations
// backed by GL sprites currently ONLY scale with viewing distance.
// annotationView.scalesWithViewingDistance = NO;
} else {
// orange indicates that the annotation view was reused
annotationView.backgroundColor = [UIColor orangeColor];
Expand Down
8 changes: 4 additions & 4 deletions platform/ios/src/MGLAnnotationView.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,12 @@ MGL_EXPORT
value of this property is `NO` or the map’s pitch is zero, the annotation view
remains the same size regardless of its position on-screen.

The default value of this property is `YES`. Set this property to `NO` if the
view’s legibility is important.
The default value of this property is `NO`. Keep this property set to `NO` if
the view’s legibility is important.
Copy link
Contributor

@1ec5 1ec5 Apr 10, 2018

Choose a reason for hiding this comment

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

Per #5040, mapbox/mapbox-gl-js#4120 (comment), and mapbox/mapbox-gl-js#4547, we don’t allow developers to turn off scaling GL point annotations or symbol layers based on viewing distance:

…or if the map also represents some annotations with MGLAnnotationImage objects, or if the style also contains symbol layers that the user can interact with.

This brings the change in behavior here into question to some extent. It’s already confusing enough that there are three ways to put a marker on the map, without one of those ways scaling different than the others.

/cc @ChrisLoer

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not familiar enough with how MGLAnnotationView is typically used to have a strong opinion here. But if I'm reading this all correctly, isn't it the case that either way, the scaling for annotation views will be 50% off from symbol layers (because they use the hardwired 50% "attenuated" pitch-scaling, and this functionality is either 0 or 100%)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I’m not sure if the pitch-scaling for view-backed annotations would be 100% – it’s a bit of a hack because we don’t have access to the map transform at this level (see also #10498). So I guess there’s inconsistency regardless. 👍


@note Scaling many on-screen annotation views can contribute to poor map
performance. Consider disabling this property if your use case involves
hundreds or thousands of annotation views.
performance. Consider keeping this property disabled if your use case
involves hundreds or thousands of annotation views.
*/
@property (nonatomic, assign) BOOL scalesWithViewingDistance;

Expand Down
2 changes: 1 addition & 1 deletion platform/ios/src/MGLAnnotationView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ - (void)commonInitWithAnnotation:(nullable id<MGLAnnotation>)annotation reuseIde
_lastAppliedScaleTransform = CATransform3DIdentity;
_annotation = annotation;
_reuseIdentifier = [reuseIdentifier copy];
_scalesWithViewingDistance = YES;
_scalesWithViewingDistance = NO;
_enabled = YES;
}

Expand Down