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

Added MapController.pointToLatLng() to get LatLng of given screen point #1115

Merged
merged 7 commits into from
May 20, 2022

Conversation

HugoHeneault
Copy link
Contributor

I wanted to get LatLng of a given point on screen, but couldn't find an easy way to.

So I've implemented a MapController.pointToLatLng() inspired by @maRci002's answer.

Ref #496, ref #607, ref #981, ref #1010

@HugoHeneault
Copy link
Contributor Author

Here's an example of what it does:
simulator_screenshot_6240A1E0-F7F7-48B6-9434-254F3B65F568

@JaffaKetchup
Copy link
Member

Fantastic! I recently manually coded this using parts from the library, but I couldn't do it easily because I didn't have access to internal variables.
This should make things much easier! Thanks.

Copy link
Collaborator

@ibrierley ibrierley left a comment

Choose a reason for hiding this comment

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

I like this, does it pass dartfmt ok ? (not sure what we're using these days, but line 86 indentation map.dart looks like it should be spaced out, but this is very minor)?

@HugoHeneault
Copy link
Contributor Author

I like this, does it pass dartfmt ok ? (not sure what we're using these days, but line 86 indentation map.dart looks like it should be spaced out, but this is very minor)?

Nope sorry, I'll dart format

Is there a way to get it done automatically?

@HugoHeneault
Copy link
Contributor Author

I just pushed a fix that caused calculation to be wrong when user zoomed the map.
Now I have an issue when user rotates the map.

RPReplay_Final1641984191.mov

Has anyone an idea?

@HugoHeneault
Copy link
Contributor Author

HugoHeneault commented Jan 13, 2022

I updated my example but still can't figure out what's wrong :(

It looks like LatLng I return doesn't take rotation in consideration. Still investigating, any help would be nice :)

Kapture.2022-01-13.at.07.58.49.mp4

@ibrierley
Copy link
Collaborator

Are you still having problems with the rotation issue ?

@HugoHeneault
Copy link
Contributor Author

Yes no updates here. Might need some help... We've - for now - disabled rotation on our app when this is active.

@ibrierley
Copy link
Collaborator

ibrierley commented Jan 26, 2022

See if this works for you....

@override
  LatLng? pointToLatLng(CustomPoint localPoint) {
    if (_state.originalSize == null) {
      return null;
    }

    final width = _state.originalSize!.x;
    final height = _state.originalSize!.y;

    var localPointCenterDistance =
        CustomPoint((width / 2) - localPoint.x, (height / 2) - localPoint.y);
    var mapCenter =
        _state.options.crs.latLngToPoint(_state.center, _state.zoom);

    var point = mapCenter - localPointCenterDistance;

    if( _state.rotation != 0.0 ) {
      var m = Matrix4.identity()
        ..translate(mapCenter.x.toDouble(), mapCenter.y.toDouble())
        ..rotateZ(-_state.rotationRad)
        ..translate(-mapCenter.x.toDouble(), -mapCenter.y.toDouble());

      var tp = MatrixUtils.transformPoint(
          m, Offset(point.x.toDouble(), point.y.toDouble()));

      return _state.options.crs.pointToLatLng(
          CustomPoint(tp.dx, tp.dy), _state.zoom);
    }

    return _state.options.crs.pointToLatLng(
        point, _state.zoom);
  }

If it does (the icon rotates around its own center, but I think either a nonrotatedwidget or an opposite rotation would probably fix), maybe the rotated matrix stuff could be broken out into a separate method, as there may be uses for it being called elsewhere....

@ibrierley
Copy link
Collaborator

Just wondering on this one, if pointToLatLng should take the option of using the maps rotation (then it could use the matrix code). As some may want to use this as a separate method sometimes for a straightforward calculation ignoring rotation.

This also needs pull upstream so we can merge whenever, as there have been some changes since.

@HugoHeneault
Copy link
Contributor Author

I'll be on vacation for two weeks, might work back on it around mi-february. Feel free to work on it during this time :)
Thanks @ibrierley

@JaffaKetchup
Copy link
Member

JaffaKetchup commented Feb 20, 2022

@HugoHeneault Any news on this? Thanks.

@HugoHeneault
Copy link
Contributor Author

No and I don't have time this days so if you'd like to investigate do not hesitate :)

@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Mar 24, 2022
@HugoHeneault
Copy link
Contributor Author

This should still be reviewed and fixed as I'm sure other people will need it...

@github-actions github-actions bot removed the Stale label Mar 26, 2022
@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Apr 25, 2022
@pablojimpas
Copy link
Contributor

@HugoHeneault would you mind re-basing this branch from master when you have time? I'll take a look at it after that.

@github-actions github-actions bot removed the Stale label May 18, 2022
@HugoHeneault
Copy link
Contributor Author

@HugoHeneault would you mind re-basing this branch from master when you have time? I'll take a look at it after that.

I'm on it :)

@JaffaKetchup
Copy link
Member

Yeah, I can't do anything directly as it's someone else's fork. I'll try fixing it through suggestions...

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

Maybe now?

lib/src/map/map.dart Outdated Show resolved Hide resolved
@HugoHeneault
Copy link
Contributor Author

If it's working, I'll apply the suggested fix tomorrow morning with formating, if you haven't figured it out already. Thanks guys for your help (I'm sorry I didn't followed the suggestion as I've moved on another project)

@JaffaKetchup
Copy link
Member

@HugoHeneault Thanks. All is needed now (I think) is just the addition of a trailing newline. Should just be able to run dart format ., or do it manually. Don't worry about doing other stuff!

@HugoHeneault
Copy link
Contributor Author

I did the reformat and tested. It works! Thanks alot @ibrierley for the solution and @pablojimpas & @JaffaKetchup for the follow-up!

LGTM :)

Co-authored-by: Polo <gitpjp@pm.me> and ibrierley 
Update lib/src/map/map.dart
Update lib/src/map/map.dart
Fix trailing lines
@JaffaKetchup
Copy link
Member

Seems good to me, thanks to:

Will merge now...

@JaffaKetchup JaffaKetchup self-requested a review May 20, 2022 16:44
Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

LGTM!

@JaffaKetchup JaffaKetchup merged commit 02b4a37 into fleaflet:master May 20, 2022
@Ronjack
Copy link

Ronjack commented May 25, 2022

wow, just in time, perfect, thanks!

@mohammedX6
Copy link

See if this works for you....

@override
  LatLng? pointToLatLng(CustomPoint localPoint) {
    if (_state.originalSize == null) {
      return null;
    }

    final width = _state.originalSize!.x;
    final height = _state.originalSize!.y;

    var localPointCenterDistance =
        CustomPoint((width / 2) - localPoint.x, (height / 2) - localPoint.y);
    var mapCenter =
        _state.options.crs.latLngToPoint(_state.center, _state.zoom);

    var point = mapCenter - localPointCenterDistance;

    if( _state.rotation != 0.0 ) {
      var m = Matrix4.identity()
        ..translate(mapCenter.x.toDouble(), mapCenter.y.toDouble())
        ..rotateZ(-_state.rotationRad)
        ..translate(-mapCenter.x.toDouble(), -mapCenter.y.toDouble());

      var tp = MatrixUtils.transformPoint(
          m, Offset(point.x.toDouble(), point.y.toDouble()));

      return _state.options.crs.pointToLatLng(
          CustomPoint(tp.dx, tp.dy), _state.zoom);
    }

    return _state.options.crs.pointToLatLng(
        point, _state.zoom);
  }

If it does (the icon rotates around its own center, but I think either a nonrotatedwidget or an opposite rotation would probably fix), maybe the rotated matrix stuff could be broken out into a separate method, as there may be uses for it being called elsewhere....

Hi @ibrierley
I want to take consideration the rotation if I want to convert from LatLng to point, How I can do it ? any hints please.

@ibrierley
Copy link
Collaborator

Are you all sorted on this now (I noted your other issue is closed), or still having an issue ?

@mohammedX6
Copy link

@ibrierley Thanks ,i got it working now.

@mohammedX6
Copy link

mohammedX6 commented Jul 26, 2022

Are you all sorted on this now (I noted your other issue is closed), or still having an issue ?

Hi, @ibrierley I am sorry to disturb you, I have been trying to achieve this for days. Without luck, I am trying here to convert LatLng Point (I need the point to be a LatLng point so Local/Global position won't work) to a point (Offset) with taking mind map rotation.
Here is what I am trying

@override
  Offset? LatLngToPoint(LatLng localPoint) {
    if (_state.originalSize == null) {
      return null;
    }

    final width = _state.originalSize!.x;
    final height = _state.originalSize!.y;

    var myLocalPoint =
        _state.options.crs.latLngToPoint(localPoint, _state.zoom) -
            _state.getPixelOrigin();

    final localPointCenterDistance = CustomPoint(
        (width / 2) - myLocalPoint.x, (height / 2) - myLocalPoint.y);
    final mapCenter =
        _state.options.crs.latLngToPoint(_state.center, _state.zoom);

    CustomPoint point = mapCenter - localPointCenterDistance;

    if (_state.rotation != 0.0) {
      point = rotatePoint(mapCenter, point);
    }
    point = point - _state.getPixelOrigin();
    return Offset(point.x.toDouble(), point.y.toDouble());
  }

can you help me, please.

@ibrierley
Copy link
Collaborator

Why do you need the latlng from screen point out of interest, normally there is a simpler way.

@mohammedX6
Copy link

mohammedX6 commented Jul 27, 2022

@ibrierley Thanks for response, I am using flutter_map_line_editor onDragUpdate callback , using the screenPoint.global This is for touch position not for the actual point position, i want the actual point position because touch position and point position are not the same.
The red circle is touch position, and the blue circle is the point while i am dragging
Screenshot (6)

@ibrierley
Copy link
Collaborator

Why are your touch and point positions not the same ?

@mohammedX6
Copy link

@ibrierley , Because the point the and the touch position will not be same

@ibrierley
Copy link
Collaborator

From the info provided, I can't really see what you need to have working beyond the original code, so I'm probably missing something.

If you want to provide a minimal example I can run, to highlight your issue, I will take a look and see if I can help.

@mohammedX6
Copy link

can we chat on the discord channel to make things easier , I will give in short time an example

@mohammedX6
Copy link

mohammedX6 commented Jul 27, 2022

@ibrierley Hi , Here is a minimal example https://filebin.net/f3na2ig0nilvp4hp/example.zip

When you start the app make sure you have rotation on the map to see the effect

@ibrierley
Copy link
Collaborator

You need to create a smaller single example, without any plugins and highlight what is wrong with pointToLatLng with a rotation.

@mohammedX6
Copy link

@ibrierley There is no wrong with pointToLatLng, I just want LatLngToPoint with rotation

@ibrierley
Copy link
Collaborator

Ok, this issue is about pointToLatLng, and the code you quoted is that :D. Probably why the confusion with no example.

Can you open a new issue that is latLngToPoint, not pointToLatLng, so it's clear what you need.

@mohammedX6
Copy link

@ibrierley yes sure

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 this pull request may close these issues.

6 participants