-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[ios] add content inset to camera edge padding #14081
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Can you add some change log blurbs please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this. Could you please add this fix to macOS as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned above, the macOS implementation of MGLMapView also has the same bug.
mbgl::CameraOptions cameraOptions = [self cameraOptionsObjectForAnimatingToCamera:camera edgePadding:edgePadding]; | ||
|
||
mbgl::EdgeInsets padding = MGLEdgeInsetsFromNSEdgeInsets(edgePadding); | ||
padding += MGLEdgeInsetsFromNSEdgeInsets(self.contentInset); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some callers are already adding the same content inset to the padding:
mapbox-gl-native/platform/ios/src/MGLMapView.mm
Line 3757 in d00d296
padding += MGLEdgeInsetsFromNSEdgeInsets(self.contentInset); |
To avoid the redundant change, those callers also need to be updated.
I haven't been able to figure out how to write a test for this case yet. Edge padding is thrown away after transform_state is finished. Perhaps a snapshot test could work. Any ideas are very much welcome. |
Is this still valid? |
Indeed, there’s been a fair bit of work done on this recently (#14664, #14795, #14813, etc.) — is this still valid, @fabian-guerra @astojilj? |
This was fixed in #14813 |
Fixes #12818
For consistency with other camera methods,
-[MGLMapView setCamera:withDuration:animationTimingFunction:edgePadding:completionHandler:]
now adds thecontentInset
to the edge padding when updating the camera.cc @fabian-guerra @julianrex @1ec5