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

Point annotations disappear at low zoomlevels #5109

Closed
tobrun opened this issue May 23, 2016 · 11 comments
Closed

Point annotations disappear at low zoomlevels #5109

tobrun opened this issue May 23, 2016 · 11 comments
Labels
Android Mapbox Maps SDK for Android bug Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS
Milestone
ios-v3.3.0

Comments

@tobrun
Copy link
Member

tobrun commented May 23, 2016

When you zoom out, point annotations disappears. Zooming back in restores them.

ezgif com-video-to-gif 20

This was noticed on Android and was validated for iOS by @1ec5 with 3.3.0-alpha.3
@tobrun tobrun added the Core The cross-platform C++ core, aka mbgl label May 23, 2016
@tobrun tobrun mentioned this issue May 23, 2016
Closed
@1ec5 1ec5 added this to the ios-v3.3.0 milestone May 23, 2016
@1ec5
Copy link
Contributor

1ec5 commented May 23, 2016

These are GL point annotations, by the way.

@bleege
Copy link
Contributor

bleege commented May 24, 2016

Discussed internally with @tobrun @cammace today and this issue is a blocker for Android 4.1.0 FINAL release.

/cc @mapbox/mobile @jfirebaugh @kkaefer @tmpsantos @brunoabinader

@bleege bleege added iOS Mapbox Maps SDK for iOS Android Mapbox Maps SDK for Android bug labels May 25, 2016
@cammace cammace mentioned this issue May 25, 2016
Closed
@tmpsantos
Copy link
Contributor

On it.

@tmpsantos
Copy link
Contributor

Found the commit that caused the regression: 4c62a55

This is not only affecting annotations but also label density on low zoom levels:
screenshot from 2016-05-27 17-27-45
screenshot from 2016-05-27 17-28-45

/cc @ansis

@bleege
Copy link
Contributor

bleege commented May 27, 2016

Awesome. Thanks @tmpsantos @ansis!

@jfirebaugh
Copy link
Contributor

@tmpsantos Any idea what about 4c62a55 is causing the issue? (Let's assume that @ansis is focused on schoolwork.)

@tmpsantos
Copy link
Contributor

tmpsantos commented May 27, 2016 •
edited
Loading

@jfirebaugh I'm investigating but seems to me some integer overflow or float to integer conversion making it full transparent.

I was initially comparing to the code on JS trying to find anything wrong with the ported code but it seems fine.

tmpsantos added a commit that referenced this issue May 27, 2016
@tmpsantos
[core] Fill the initial changeTimes with the reference timestamp of…
9db75f1 
… the first frame

On the Javascript implementation the array is automatically filled with
zeroes which is not the same thing as `TimePoint::min()`. Here we need
to get our own "zero" which is the timestamp of the first frame.

Fixes #5109
@tmpsantos tmpsantos mentioned this issue May 27, 2016
Merged
@bleege
Copy link
Contributor

bleege commented May 27, 2016

@tmpsantos I just tested your fix in #5175 and can confirm that it solves this issue. Both markers (C and A) were visible at all zoom levels as well as when cycled on / off using the FAB (see screenshots below). I'm going to merge #5175 into master and then cherry pick the commit over to the release-android-v4.1.0 branch so that this fix gets into the nightly SNAPSHOTS as well as the final release. Thank you for fixing this!

device-2016-05-27-160756

device-2016-05-27-160811

bleege pushed a commit that referenced this issue May 27, 2016
@tmpsantos @bleege
[core] Fill the initial changeTimes with the reference timestamp of…
623b81f 
… the first frame

On the Javascript implementation the array is automatically filled with
zeroes which is not the same thing as `TimePoint::min()`. Here we need
to get our own "zero" which is the timestamp of the first frame.

Fixes #5109
@bleege
Copy link
Contributor

bleege commented May 27, 2016

Cherry picked into Android's release-android-v4.1.0 branch.

screen shot 2016-05-27 at 4 21 41 pm

@mpuchala
Copy link

@bleege @tobrun
When can we expect new snapshot or beta-2 release with this fix?

@tobrun
Copy link
Member Author

tobrun commented May 30, 2016

@mpuchala, thank you for reaching out. In normal circumstances you should be able to use the SNAPSHOT containing this fix. I just double checked the nightly builds and there seems to be a problem with building. I'm fixing this in #5187.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Android Mapbox Maps SDK for Android bug Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS
Projects
None yet
Milestone
ios-v3.3.0
Development

No branches or pull requests
6 participants
@jfirebaugh @bleege @tmpsantos @1ec5 @tobrun @mpuchala