Make annotation view rotation alignment configurable #8308
Conversation
This commit adds `rotatesWithMap` property on `MGLAnnotationView`. This property, when set to `YES` fixes the annotation to a map such that view follows map's rotation angle. This is useful when user wants to display rotation-dependent annotations (e.g. sector lights). Defaults to `NO`. Encoded.
eimantas
changed the title
rotatesWithMaprotatesWithMap
eimantas
changed the title
rotatesWithMaprotatesWithMap
There was a problem hiding this comment.
This is related to #5245. Synchronizing the rotation on its own, as this PR does, is a more manageable slice of the problem. However, the visual effect when the map is tilted is still poor unless we expose a usable transform matrix to SDK code. Perhaps that shouldn't block this feature, since we've had no movement on the grander feature in #5245 for some time.
platform/ios/src/MGLAnnotationView.h
Outdated
| The default value of this property is `NO`. Set this property to `YES` if the | ||
| view’s rotation is important. | ||
| */ | ||
| @property (nonatomic, assign) BOOL rotatesWithMap; |
There was a problem hiding this comment.
#5245 at one point called this property rotatesToMatchCamera, which I think would do a better job of describing exactly what it does. (After all, technically you could rotate the MGLMapView itself in 3D space! 😂)
|
I actually spent some time trying to match the tilting too, but didn't manage to get the transform matrix calculations right so just came with this simple implementation since that was enough for the project I was working on (no tilting was enabled on the map). The exposed transform from core to sdk would definitely make things easier. |
1ec5
added
annotations
1ec5
changed the title
rotatesWithMap
platform/ios/src/MGLAnnotationView.h
Outdated
| @@ -141,6 +141,19 @@ typedef NS_ENUM(NSUInteger, MGLAnnotationViewDragState) { | |||
| */ | |||
| @property (nonatomic, assign) BOOL scalesWithViewingDistance; | |||
|
|
|||
| /** | |||
| A Boolean value that determines whether the annotation view rotates together | |||
| with the mapview. | |||
There was a problem hiding this comment.
Nit: replace “mapview” with “map”, since the direction property reflects the rotation of the content within the map view, not the map view itself.
| } | ||
| } | ||
|
|
||
| - (void)updateRotateTransform |
There was a problem hiding this comment.
As far as I can tell, this method only gets called when rotatesToMatchCamera is set explicitly by application code. But the annotation view’s transform matrix is always overridden whenever the map viewport changes. -updateScaleTransformForViewingDistance gets called by -setCenter:, which coincidentally happens to be the method that -[MGLMapView update AnnotationViews] calls to clobber reposition the annotation view to match the map. We could merge this method and -updateScaleTransformForViewingDistance into an -updateTransform method that comes up with the canonical transform to apply.
There was a problem hiding this comment.
Right. I was copy-pasting the code from my private repository that was used with the project. Forgot to add the call to [self updateRotateTransform]; from setCenter.
| The default value of this property is `NO`. Set this property to `YES` if the | ||
| view’s rotation is important. | ||
| */ | ||
| @property (nonatomic, assign) BOOL rotatesToMatchCamera; |
There was a problem hiding this comment.
Thanks for making this change. FYI, I was torn between suggesting rotatesToMatchCamera and suggesting a rotationAlignment property of type MGLIconRotationAlignment, for consistency with MGLSymbolStyleLayer. But I think a Boolean rotatesToMatchCamera will end up being more intuitive for developers, since we have no need to support an “auto” value here.
|
I guess this won't make it to 3.5.0? |
|
Unfortunately, we’re unable to take additional features for v3.5.0 due to the condensed schedule for that release, but I put this PR on the v3.6.0 milestone to make sure it doesn’t fall off our radar. |
|
This PR is in a strange state where GitHub seems to think the fork has been deleted (see I just rebased on top of the After my commit (10c4acb), the behavior looks like the gif below which I think is what we want.
I'd like to get another round of review on this and merge it next week. However, with the PR in the state it is in now, I think I may need to redo all of these changes in a new PR or @eimantas might need to resubmit the PR from a new fork. |
|
Continuing in #9147 |
This PR adds
rotatesWithMapproperty toMGLAnnotationView. Thisproperty, when set to
YESfixes the annotation to a map such that viewfollows map's rotation angle. This is useful when user wants to display
rotation-dependent annotations (e.g. sector lights).
Defaults to
NO. Encoded.