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

#2151 MGLAnnotation centerOffset and calloutOffset properties #3220

Closed
wants to merge 4 commits into from

Conversation

RomainQuidet
Copy link
Contributor

Add centerOffset and calloutOffset properties to MGLAnnotation
for issue #2151

[self.mapView removeAnnotations:self.mapView.annotations];
NSInteger settingChoice = buttonIndex - 1; // cancel is index 0
switch (settingChoice) {
case settingIndexResetNorth:
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, the switch statement is a good idea here.

@1ec5
Copy link
Contributor

1ec5 commented Dec 8, 2015

Getting the annotation image for an annotation is a couple simple lookups (first get the metadata dictionary for the annotation, then "dequeue" the annotation's reusable image). Since the callout is an iOS-specific implementation, it doesn't seem necessary to add a parallel field in mbgl code. @incanus, would that be desirable anyways?

@RomainQuidet
Copy link
Contributor Author

all callout offset update is only within iOS code. The only update needed was to take into account the annotation image center offset to position by default the callout y on the top of the image.
The annotation image offset needs to go deep into mgl core as it is rendered here.

@friedbunny friedbunny changed the title 2151 #2151 MGLAnnotation centerOffset and calloutOffset properties Dec 8, 2015
@1ec5
Copy link
Contributor

1ec5 commented Dec 8, 2015

Ah, makes sense then. 👍 Although I wonder what should happen if the UIImage has an alignment rect set. (On OS X, I’m using the alignment rect to help determine the hit target and callout offset.)

@1ec5 1ec5 added feature iOS Mapbox Maps SDK for iOS labels Dec 8, 2015
@1ec5 1ec5 added this to the ios-v3.1.0 milestone Dec 8, 2015
@RomainQuidet
Copy link
Contributor Author

You're right. The offset reveal a conception problem: the annotation touch event is not "really" linked to the image but to an arbitrary area around the geographical position. It's mostly working, but will fail if the offset is too hight or left/right.
I think this part needs a refactor. How would you like me to help ? What's the better way to work with you on this ?

@RomainQuidet
Copy link
Contributor Author

the tap issue is #1504

@1ec5
Copy link
Contributor

1ec5 commented Dec 9, 2015

Correcting the hit testing will require #3159. It’s a big change, but I’ve done it before on the iOS side as part of #1496, which unfortunately went stale. I may have time either today or tomorrow to work on it, depending on how my other active PRs progress. Once that’s done, either of us can implement the approach taken in 05b039e for #3135. (See -installAnnotationImage: and -handleClickGesture:.) I do think your approach is better in terms of not requiring the developer to manually pad the image with whitespace. If we can combine that with the hit testing improvements in 05b039e, I think we’ll be in good shape for when #837 lands and additional properties become accessible.

@RomainQuidet
Copy link
Contributor Author

Ok, feel free to ask me when you need help on iOS / Core, I'm dedicated by Mappy company to help on that part.

@1ec5
Copy link
Contributor

1ec5 commented Dec 16, 2015

SpriteImage assumes that images can only have integral width and height. I attempted to introduce the concept of floating-point width and height in #3008, but it turns out that we need to refactor more of mbgl to always work with pixel dimensions (which would be integral regardless) instead of point dimensions. We’re tracking this work in #3164.

/cc @jfirebaugh

@1ec5
Copy link
Contributor

1ec5 commented Jan 18, 2016

Once #3561 lands, SpriteImage will be expressed entirely in terms of premultiplied pixels (backing pixels in Cocoa terminology). That’ll clear the way for this PR.

@1ec5
Copy link
Contributor

1ec5 commented Jan 22, 2016

@jfirebaugh pointed out that we should take better advantage of the GL style system for the center offset functionality:

If we want to offer this functionality, it should use the icon-offset style property. But in order to control that per-annotation, we need either data-driven styles or control over annotation grouping into layers.

The callout offset functionality doesn’t fit into the style system, and it’s implemented entirely within the iOS SDK, so that isn’t a concern. But I think I’d still like to see it expressed as a calloutAnchorEdge property of type UIRectEdge. (We already have support for aligning the callout view based on the image’s alignment insets, in case you need finer-grained control over the anchor’s placement.)

@1ec5 1ec5 modified the milestones: ios-v3.2.0, ios-v3.1.0 Jan 22, 2016
@RomainQuidet
Copy link
Contributor Author

I updated the pull request to work on v3.2.0

@1ec5
Copy link
Contributor

1ec5 commented Jul 5, 2016

centerOffset was added to MGLAnnotationView in #5062. Still waiting on the runtime styling API before tackling this for MGLAnnotationImage.

@boundsj boundsj modified the milestones: ios-v3.4.0, ios-future Jul 11, 2016
@tmpsantos
Copy link
Contributor

Stale.

@tmpsantos tmpsantos closed this Oct 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants