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

avoid altering annotation view transforms set by user #6166

Merged
merged 1 commit into from
Aug 26, 2016

Conversation

incanus
Copy link
Contributor

@incanus incanus commented Aug 26, 2016

I unearthed a bug where trying to set an annotation view transform of your own gets rapidly reset nearly every frame while annotation view center is changing due to the CATransform3DIdentity that is repeatedly set in our setCenter: override.

One case this doesn't cover is when you do have draggable views but still want to set a transform. Perhaps we could create an alternate setter called setCenterDuringAnnotationUpdate: which is called by -[MGLMapView updateAnnotationViews] to bypass any of this drag-related transform setting entirely?

@1ec5 @friedbunny @boundsj

@incanus incanus added bug iOS Mapbox Maps SDK for iOS annotations Annotations on iOS and macOS or markers on Android MapKit parity For feature parity with MapKit on iOS or macOS labels Aug 26, 2016
@incanus incanus self-assigned this Aug 26, 2016
@mention-bot
Copy link

@incanus, thanks for your PR! By analyzing this pull request, we identified @boundsj, @1ec5 and @friedbunny to be potential reviewers.

@incanus incanus added this to the ios-v3.4.0 milestone Aug 26, 2016
@1ec5
Copy link
Contributor

1ec5 commented Aug 26, 2016

-updateTransform isn’t about dragging (and is already a no-op while dragging). It’s about scalesWithViewingDistance, which is on by default and which this PR breaks. The method is also set to get more complex once #5245 lands.

@1ec5
Copy link
Contributor

1ec5 commented Aug 26, 2016

If resetting to CATransform3DIdentity is a problem, one solution would be:

  1. Keep track of the CATransform3D that we apply within -updateTransform.
  2. Every time we call -updateTransform, reverse the previously-applied transform and apply the new one, without ever resetting to CATransform3DIdentity.

@incanus
Copy link
Contributor Author

incanus commented Aug 26, 2016

Yeah I am thinking the manual bookkeeping might be necessary. Will keep on this.

@incanus incanus force-pushed the fix-ios-annotation-view-transforms branch from 1fbfc73 to c0edb62 Compare August 26, 2016 19:20
@incanus
Copy link
Contributor Author

incanus commented Aug 26, 2016

I think I fixed things in c0edb62. However @1ec5 I would also appreciate an eye on 51aeb89 to make sure that I didn't lose some intended effect by simply getting rid of the unconditional identify transform there.

@1ec5
Copy link
Contributor

1ec5 commented Aug 26, 2016

I think that should be OK. Incidentally, CALayer.sublayerTransform seems purpose-made for our intended effect, but it’s iOS 8+ API.

@1ec5
Copy link
Contributor

1ec5 commented Aug 26, 2016

Also, would you do the honors and update the changelog?

@incanus incanus changed the title avoid altering annotation view transforms if not draggable avoid altering annotation view transforms set by user Aug 26, 2016
@incanus
Copy link
Contributor Author

incanus commented Aug 26, 2016

Not sure why Android test is failing, but going to rebase on master and merge this.

@incanus incanus force-pushed the fix-ios-annotation-view-transforms branch from d04746c to 2501b2c Compare August 26, 2016 21:30
@incanus incanus merged commit 2501b2c into master Aug 26, 2016
@incanus incanus deleted the fix-ios-annotation-view-transforms branch August 26, 2016 21:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
annotations Annotations on iOS and macOS or markers on Android bug iOS Mapbox Maps SDK for iOS MapKit parity For feature parity with MapKit on iOS or macOS needs discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants