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

[ios] Avoid drawing view annotations across pixel boundaries #10219

Merged
merged 3 commits into from
Oct 19, 2017

Conversation

friedbunny
Copy link
Contributor

@friedbunny friedbunny commented Oct 18, 2017

Fixes #10119 by rounding view anchor points so that annotation views are not being drawn across pixel boundaries. This change impacts how all view annotations are drawn, including the user location annotation, regular view annotations, and also callout views.

In most cases, the misalignment was being introduced by -convertCoordinate:toPointToView:, which converts from geographic coordinates to screen points, but with floating point precision. This PR leaves the precision of that method intact, but rounds the returned point value to the nearest pixel before we use it to place the view.

Note that MGLPointRounded() will round to pixels and not necessarily points — it’s fine to draw to logical pixels on scaled displays.

Also fixed is the misalignment of the scale bar labels — these should be crisper now, too.

/cc @1ec5 @frederoni @fabian-guerra @boundsj

@friedbunny friedbunny added annotations Annotations on iOS and macOS or markers on Android iOS Mapbox Maps SDK for iOS labels Oct 18, 2017
@friedbunny friedbunny added this to the ios-v3.7.0 milestone Oct 18, 2017
@friedbunny friedbunny self-assigned this Oct 18, 2017
@friedbunny
Copy link
Contributor Author

macOS doesn’t have UIScreen, so I’ll have to replace that with something more Darwin-friendly.

Copy link
Contributor

@fabian-guerra fabian-guerra left a comment

Choose a reason for hiding this comment

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

How the annotation's location accuracy will be affected at different zoom levels?

@friedbunny
Copy link
Contributor Author

friedbunny commented Oct 18, 2017

How the annotation's location accuracy will be affected at different zoom levels?

@fabian-guerra There are two scenarios, but in both the accuracy should remain unchanged (as this is always sub-pixel rounding of a screen point, just after it was converted from a geographic coordinate).

  1. A user tracking mode is enabled and the user location annotation remains fixed at a constant point on the screen. The original placement point is unaffected by zoom and gets rounded once.
  2. User tracking is disabled or this is a regular view annotation — the relative position of the view annotation is recalculated from its coordinate whenever the map moves, in which case the new position is rounded to the nearest pixel.

In both cases, the original geographic coordinate remains untouched and only the screen point at which the view annotation is placed is modified. At most, the difference introduced by the rounding will be ~0.5 pixels, which does not affect accuracy and subtly improves rendering sharpness.

@friedbunny friedbunny force-pushed the fb-user-dot-pedantry branch from bc87592 to 23bd12e Compare October 18, 2017 18:12
Copy link
Contributor

@fabian-guerra fabian-guerra left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@friedbunny friedbunny force-pushed the fb-user-dot-pedantry branch from 23bd12e to 0e61a82 Compare October 18, 2017 23:20
@friedbunny friedbunny force-pushed the fb-user-dot-pedantry branch from 0e61a82 to 817e5c1 Compare October 19, 2017 15:03
@friedbunny friedbunny merged commit 817e5c1 into release-agua Oct 19, 2017
@friedbunny friedbunny deleted the fb-user-dot-pedantry branch October 19, 2017 15:48
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 iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants