Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MapAnnotations not centered properly #14

Open
rderimay opened this issue Jun 28, 2022 · 32 comments · May be fixed by #53
Open

MapAnnotations not centered properly #14

rderimay opened this issue Jun 28, 2022 · 32 comments · May be fixed by #53

Comments

@rderimay
Copy link

Not sure if the problem comes from your package or from MapKit directly. Whenever I add MapAnnotations, these are not centered on their middle point and rotate around another near point when rotating the map

Screen 2022-06-28 à 23 40 05

The crazy thing is that this does not always happen. Rarely, after the app being compiled, it works perfectly...

@pauljohanneskraft
Copy link
Owner

I'm not entirely sure why this is. I have experienced minor issues in this regard, but this seems to be off pretty much for all of the annotations and not even in the same direction, right? Could you maybe provide a minimal example project experiencing this issue, so that I could debug this? Would be greatly appreciated.

@rderimay
Copy link
Author

This is even stranger actually because sometimes when the app stars, this is ok, and and once when swapping maps back and forth, it becomes twisted with no change to the underlying data...

@pauljohanneskraft
Copy link
Owner

It's quite hard to predictably reproduce this issue for me - but I have a promising idea. Could you please try #18 whether it fixes that behavior?
If not, do you have some minimal example that I could use for debugging purposes? Would be much appreciated 😊

@pauljohanneskraft
Copy link
Owner

I just tried the most recent change on that branch and it worked without the issue appearing at all - if you notice this issue persists in future versions of Map, please let me know that this still persists/reappeared!

Closing this for now - will reopen, if the issue persists.

@pauljohanneskraft
Copy link
Owner

Reopening, since it doesn't seem to be fully solved. I was still able to reproduce this issue in certain cases. Not really sure what the issue here is - seems to be somewhat related to safe area insets, maybe?

@rderimay
Copy link
Author

Can also confirm that the last commit does not solve this issue.

@rderimay
Copy link
Author

I've created a stripped down test project where I can reproduce the problem. How can I send it to you @pauljohanneskraft ?

@pauljohanneskraft
Copy link
Owner

pauljohanneskraft commented Jul 12, 2022

Thank you very much! You can send it as a zip right here by drag-and-dropping it in the text field, if you want.

@rderimay
Copy link
Author

rderimay commented Jul 12, 2022

Tester.zip

Here you go @pauljohanneskraft . You'll see that the white dots are not properly centered on the track (that could be me not setting them correctly), but they also rotate around a center slightly outside of them when you rotate the view for example. This offset is not constant from one app start to another...

@pauljohanneskraft
Copy link
Owner

Thank you very much! 😊

@alex-reilly-pronto
Copy link

alex-reilly-pronto commented Jul 14, 2022

I've also been experiencing this issue, but it doesn't happen for all annotations. During the same run, some will be placed correctly, and some will be off. Creating a minimal example isn't going to be possible for me, but I'm down to try things.

@pauljohanneskraft
Copy link
Owner

@alex-reilly-pronto Thank you very much!

So what I have already figured out in the example above is, that the backwards support for MKAnnotations is used. In that case, the MKAnnotation instances should not be recreated during every run (this should probably be added to the Readme), since that would mean that in every run, the annotations are removed and the newly created ones are added to the underlying MKAnnotationView. You can get around this by simply creating @State properties in that view that are created the same way as they currently are in the initializer.

In addition to that, there seems to be an issue with ignoring the safe area - I will have a look at that as well. It seemingly only appears once the first one is "fixed". If you do not ignore the safe area for now, they should be at the place where they belong (which is ofc not a satisfactory solution, I know).

@eugenijusr
Copy link

Is this maybe because ViewMapAnnotation is missing the anchorPoint property? If you don't specify it for MapAnnotation you have the same behavior - pins drifting as you zoom because of improper alignment. Specifying anchorPoint fixes it:

MapAnnotation(coordinate: coordinate, anchorPoint: .init(x: 0.5, y: 1.0)) {
  MyPinView()
}

ViewMapAnnotation doesn't have this property.

@pauljohanneskraft
Copy link
Owner

pauljohanneskraft commented Jul 30, 2022

@eugenijusr It is right that the anchorPoint functionality is still missing here, it is not causing the issue though, as I pretty much hard code anchorPoint to be .init(x: 0.5, y: 0.5) - you can still get the behavior of the original version by making your view larger at the moment. I know that this is not exactly the same behavior, but it is close and should work for the most cases in the mean time.

@alex-reilly-pronto and @rderimay It would be really nice, if you could try to put .fixedSize() on the views that you are using inside the ViewMapAnnotation and also make sure to not recreate the MKAnnotation/MKCircle objects on each execution of the body (e.g. by wrapping it into an @State. With the onChange view modifier, you can still recreate this value on change of any underlying values.). Please let me know, if this solves the issue.

@rderimay
Copy link
Author

@pauljohanneskraft I changed MKAnnotation to a @State being calculated onAppear as they don't change over time, this is no problem.

As for the fixedSize() / .edgesIgnoringSafeArea(.all), I am having no success.
Removing .edgesIgnoringSafeArea(.all) completely doesn't even change the problem. I makes the offset smaller maybe, but that's all. I also tried to leave .edgesIgnoringSafeArea(.all) and to add .fixedSize() with a fixed frame(width:height:) for testing, but it does not change the behaviour.
Hope you can find the root of the problem soon...

@BucekJiri
Copy link

We developed a similar library to yours. We also placed the view for the annotation into a HostingController and then added the HostingController as s subview to the MKAnnotationView. And we observed exactly the same bug with misplaced annotations.

This was solved by using the MKAnnotationView.image property instead of adding the view as a subview. The challenge here is transforming the view into a UIImage. I believe this can be done using an ImageRenderer (only for iOS 16) or passing a UIImage instead of a View somehow in the constructor.

We did not fix this fully and finish the library ourselves but maybe this info could help you somehow.

@jose-roberto-abreu
Copy link

We developed a similar library to yours. We also placed the view for the annotation into a HostingController and then added the HostingController as s subview to the MKAnnotationView. And we observed exactly the same bug with misplaced annotations.

This was solved by using the MKAnnotationView.image property instead of adding the view as a subview. The challenge here is transforming the view into a UIImage. I believe this can be done using an ImageRenderer (only for iOS 16) or passing a UIImage instead of a View somehow in the constructor.

We did not fix this fully and finish the library ourselves but maybe this info could help you somehow.

I'm experiencing the same issue, only when the MKMapAnnotationView is used in conjunction with the HostingController (MKMarkerAnnotationView is working fine). The downside of converting to an UIImage, is that will lose the animation that I have set up on the SwiftUI View (i.e like pulse effect)

@alex-reilly-pronto
Copy link

Have folks been able to predictably reproduce the bug? Could the annotation be manually offset to counteract it?

@jose-roberto-abreu
Copy link

jose-roberto-abreu commented Aug 4, 2022

I can reproduce consistently with the following piece of code (as we zoom out, the circle shows up out of their coordinate center - the SwiftUI's Map is working fine in this scenario, in fact using a MKMarkerAnnotationView also works fine):

struct MapLocation: Identifiable {
    var id: String
    var coordinate: CLLocationCoordinate2D
}

struct ContentView: View {
    @State var locations: [MapLocation] = [
        MapLocation(id: "1", coordinate: CLLocationCoordinate2D(latitude: 18.473194, longitude: -69.934478))
    ]
    
    @State private var coordinateRegion = MKCoordinateRegion(
        center: .init(latitude: 18.478541, longitude: -69.924361),
        span: .init(latitudeDelta: 0.01, longitudeDelta: 0.01)
    )
    
    var body: some View {
        Map(coordinateRegion: $coordinateRegion, annotationItems: locations) { location in
            ViewMapAnnotation(coordinate: location.coordinate) {
                Circle()
                    .fill(.red)
                    .frame(width: 25, height: 25)
            }
        }
        .preferredColorScheme(.dark)
        .ignoresSafeArea()
    }
}

One note:

  • In the MKMapAnnotationView, The controller's preferredContentSize is always zero (somebody already opens a radar https://openradar.appspot.com/FB7650894). So, in this case, should be better to use the controller.view.intrisincContentSize or controller.view.subviews.first?.bounds.size

@jose-roberto-abreu
Copy link

Updating the MKMapAnnotationView's setup method to this, has an interesting effect:

func setup(for mapAnnotation: ViewMapAnnotation<Content>) {
        annotation = mapAnnotation.annotation

        let controller = NativeHostingController(rootView: mapAnnotation.content)
        addSubview(controller.view)
        
        controller.view.sizeToFit()
        bounds.size = controller.view.intrinsicContentSize
        
        self.controller = controller
    }

IMG_5BA97A909324-1

^ The black square is the controller's view and it has the right position (no matter if you zoom in or out), but for some reason the content (SwiftUI View) is misplaced.

@jose-roberto-abreu
Copy link

jose-roberto-abreu commented Aug 4, 2022

I think I found a solution, appliying this two changes on the MKMapAnnotationView make it work:

// MARK: Methods

    func setup(for mapAnnotation: ViewMapAnnotation<Content>) {
        annotation = mapAnnotation.annotation

        let controller = NativeHostingController(rootView: mapAnnotation.content)
        addSubview(controller.view)
        
        controller.view.sizeToFit()
        bounds.size = controller.view.intrinsicContentSize
        
        self.controller = controller
    }

    // MARK: Overrides

    override func layoutSubviews() {
        super.layoutSubviews()

        if let controller = controller {
            controller.view.subviews.forEach({ view in view.frame = controller.view.bounds })
        }
    }

^ This help to keep the annotation view position. But in my case, my SwiftUI's view has a pulse effect (and the ring circles are misplaced, for some reason those circles didn't reposition)

@BucekJiri
Copy link

This completely breaks the subviews of your custom View that you pass in the ViewMapAnnotation.content property. It resizes the subviews to fill the HostingController bounds. I only see resized colored rectangles.

@jose-roberto-abreu
Copy link

In the end, the changes that are working for me (including the animation on SwiftUI content) are this:

MKMapAnnotationView

    func setup(for mapAnnotation: ViewLocationDotMapAnnotation<Content>) {
        annotation = mapAnnotation.annotation
        let controller = UIHostingController(rootView: mapAnnotation.content())
        controller.view.backgroundColor = .clear
        addSubview(controller.view)
        
        controller.view.translatesAutoresizingMaskIntoConstraints = false
        NSLayoutConstraint.activate([
            controller.view.centerXAnchor.constraint(equalTo: centerXAnchor),
            controller.view.centerYAnchor.constraint(equalTo: centerYAnchor)
        ])
        
        bounds.size = controller.view.subviews.max(by: { $0.bounds.contains($1.bounds) })?.bounds.size ?? .zero
        self.controller = controller
    }

    override func layoutSubviews() {
        super.layoutSubviews()
        bounds.size = controller?.view?.subviews.max(by: { $0.bounds.contains($1.bounds) })?.bounds.size ?? .zero
    }

@BucekJiri
Copy link

BucekJiri commented Aug 8, 2022

@Abreu0101 Thanks for this! I had to add the top and bottom constraints as well to make it work for my use case but then it worked all fine 👍

 func setup(for mapAnnotation: ViewLocationDotMapAnnotation<Content>) {
        annotation = mapAnnotation.annotation
        let controller = UIHostingController(rootView: mapAnnotation.content())
        controller.view.backgroundColor = .clear
        addSubview(controller.view)

        controller.view.translatesAutoresizingMaskIntoConstraints = false
        NSLayoutConstraint.activate([
            controller.view.centerXAnchor.constraint(equalTo: centerXAnchor),
            controller.view.centerYAnchor.constraint(equalTo: centerYAnchor),
            controller.view.topAnchor.constraint(equalTo: topAnchor),
            controller.view.bottomAnchor.constraint(equalTo: bottomAnchor)
        ])

        bounds.size = controller.view.subviews.max(by: { $0.bounds.contains($1.bounds) })?.bounds.size ?? .zero
        self.controller = controller
    }

    override func layoutSubviews() {
        super.layoutSubviews()
        bounds.size = controller?.view?.subviews.max(by: { $0.bounds.contains($1.bounds) })?.bounds.size ?? .zero
    }

@BucekJiri
Copy link

@Abreu0101 I take back what I wrote above. Your code made it better but did not solve the issue entirely for me. Upon investigating a bit closer I found out that the UIHostingController applies some random SafeAreaInsets to the misplaced annotations. This is not an uncommon bug, you can read on it here.

You need to disable the SafeArea behaviour for the UIHostingController. There is no native API for now. So the code that made the trick for me eventually is:

    private func updateContent(for selectedState: Bool) {
        guard let contentView = selectedState ? viewMapAnnotation?.selectedContent : viewMapAnnotation?.content else {
            return
        }
        controller?.view.removeFromSuperview()
        let controller = NativeHostingController(rootView: contentView, ignoreSafeArea: true)
        addSubview(controller.view)
        bounds.size = controller.preferredContentSize
        self.controller = controller
    }
    
    extension UIHostingController {
        /// This convenience init uses dynamic subclassing to disable safe area behaviour for a UIHostingController
        /// This solves bugs with embedded SwiftUI views having redundant insets
        /// More on this here: https://defagos.github.io/swiftui_collection_part3/
        /// - Parameters:
        ///   - rootView: The content View
        ///   - ignoreSafeArea: Disables the safe area insets if true
        convenience public init(rootView: Content, ignoreSafeArea: Bool) {
            self.init(rootView: rootView)
            
            if ignoreSafeArea {
                disableSafeArea()
            }
        }
        
        func disableSafeArea() {
            guard let viewClass = object_getClass(view) else { return }
            
            let viewSubclassName = String(cString: class_getName(viewClass)).appending("_IgnoreSafeArea")
            if let viewSubclass = NSClassFromString(viewSubclassName) {
                object_setClass(view, viewSubclass)
            }
            else {
                guard let viewClassNameUtf8 = (viewSubclassName as NSString).utf8String else { return }
                guard let viewSubclass = objc_allocateClassPair(viewClass, viewClassNameUtf8, 0) else { return }
                
                if let method = class_getInstanceMethod(UIView.self, #selector(getter: UIView.safeAreaInsets)) {
                    let safeAreaInsets: @convention(block) (AnyObject) -> UIEdgeInsets = { _ in
                        return .zero
                    }
                    class_addMethod(viewSubclass, #selector(getter: UIView.safeAreaInsets),
                                    imp_implementationWithBlock(safeAreaInsets),
                                    method_getTypeEncoding(method))
                }
                
                objc_registerClassPair(viewSubclass)
                object_setClass(view, viewSubclass)
            }
        }
    }

@jose-roberto-abreu
Copy link

@BucekJiri Thanks for sharing that article, that explain a lot of things. 👏🏼

@alex-reilly-pronto
Copy link

@BucekJiri Your patch looks like it's working for me :) Thanks!

@BucekJiri
Copy link

Btw you could just do controller._disableSafeArea = true but I am not sure this would make it through the App Store review since it is a private API and they probably scan for these.

@rderimay
Copy link
Author

@BucekJiri I can not get where to put you updateContent function in my SwiftUI view to make it working. If you could explain, it would be very nice. Thanks !

@BucekJiri
Copy link

@rderimay You can check the PR for annotation selection where you can see how this is used in the MKMapAnnotationView.
This is not needed to fix the issue with incorrectly centered annotations though. The changes that fixed it have been already merged in this commit right before you asked.

@rderimay
Copy link
Author

@BucekJiri Thanks! I saw this but as there is no PR for the centring fix, I was a bit unsure how to integrate that in the code actually. I will probably make a fork to integrate it in my code quickly unless there is a better solution...

@Terabyte1385
Copy link

Updating the MKMapAnnotationView's setup method to this, has an interesting effect:

func setup(for mapAnnotation: ViewMapAnnotation<Content>) {
        annotation = mapAnnotation.annotation

        let controller = NativeHostingController(rootView: mapAnnotation.content)
        addSubview(controller.view)
        
        controller.view.sizeToFit()
        bounds.size = controller.view.intrinsicContentSize
        
        self.controller = controller
    }

IMG_5BA97A909324-1

^ The black square is the controller's view and it has the right position (no matter if you zoom in or out), but for some reason the content (SwiftUI View) is misplaced.

I'm experiencing the same thing. My assumption is that the circle in your screenshot is indeed centered, but centered inside the safe area, while the map view extends outside of it.

@darronschall darronschall linked a pull request Oct 2, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants