Proposed fix for unsymmetrical anchored markers disappearing #1291

aytunch · 2022-06-28T21:50:38Z

@ibrierley @JaffaKetchup please check and see if there is any hiding happening with your working projects.
Thank you very much for the discussion.

Fixes #918.

aytunch · 2022-06-28T22:05:56Z

I will enter one more commit. There is an error

aytunch · 2022-06-28T22:29:08Z

Even after the commit, my markers are disappearing. I will investigate a little more...

aytunch · 2022-06-29T00:23:35Z

Ok friends. I fixed the math and tested it.
In the video you can see that the logs inside of the if (!map.pixelBounds.containsPartialBounds(Bounds(sw, ne))) condition fires whenever a Marker is out of bounds. Even when the Anchor point is not symmetrical.
I would be very happy if you can test and merge the PR if you find fit.
Thanks

flutter_map_marker_hide_fix.mp4

ibrierley

Haven't had time to test, but I note there's a typo in "right vs rigth"

aytunch · 2022-06-29T10:08:41Z

Haven't had time to test, but I note there's a typo in "right vs rigth"

I always confuse them :) nice catch

ibrierley

So, coordinate systems headache incoming, related to yesterdays comment :D

Are we sure 0,0 is bottom/right ? That sounds totally odd. (This isn't your fault btw, original is like this), but I can't work out if it should be different or not, I think it does work with it all swapped around).

Eg Why is sw.x larger than ne.x ? I feel like some stuff is still reversed :D so I'm finding it hard to approve this change when my spider sense is tingling.

aytunch · 2022-06-29T13:37:40Z

I am sure that for Anchor coordinate system, the (0,0) is bottom right.

When I printed these values, I saw that Anchor.left is 113 when the width is 149
However my anchor is positioned closer to the left side of the Marker

Similarly Anchor.top prints 0 and the anchor is on the bottom of the Marker

THREE ICON
flutter: 3_Activity_Marker--------marker.anchor.left: 113.112--------marker.anchor.top: 0.0
flutter: 3_Activity_Marker--------marker.width: 149.112-------------marker.height: 81.0
TWO ICON
flutter: 2_Activity_Marker--------marker.anchor.left: 74.556--------marker.anchor.top: 0.0
flutter: 2_Activity_Marker--------marker.width: 110.556------------ marker.height: 81.0

aytunch · 2022-06-29T13:44:35Z

@ibrierley for the namings, I agree with you. They can cause confusion.
I can swap right and left names and top and bottom naming.
But the calculations are true. You can see in the video as well.

Would this make more sense? (I swapped left/right and top/bottom) This way sw and ne makes more sense

          final rightPortion = marker.width - marker.anchor.left;
          final leftPortion = marker.anchor.left;
          final bottomPortion = marker.height - marker.anchor.top;
          final topPortion = marker.anchor.top;

          final sw =
              CustomPoint(pxPoint.x + leftPortion, pxPoint.y - bottomPortion);
          final ne =
              CustomPoint(pxPoint.x - rightPortion, pxPoint.y + topPortion);

          if (!map.pixelBounds.containsPartialBounds(Bounds(sw, ne))) {
            continue;
          }
          .............
          Positioned(
              key: marker.key,
              width: marker.width,
              height: marker.height,
              left: pos.x - rightPortion,
              top: pos.y - bottomPortion,
              child: markerWidget,
            ),

I feel like, whatever we change, it will still not make too much sense because we are dealing with 2 different coordinate systems in the same code snippet. Map coordinate system having (0,0) at top-left and Anchor coordinate system having (0,0) at bottom-right

@JaffaKetchup I know you are busy with the exams. But can you at least confirm this is the case with the coordinate systems? And good luck with the grades ;)

ibrierley · 2022-06-29T14:09:15Z

(btw I'm away from tomorrow for a few days, and as Jaffa may be busy, it might be a case of revisiting next week, I suspect I don't have any issue at all with the fix, just maybe it needs some extra code docs or something)

aytunch · 2022-06-29T14:22:51Z

@ibrierley ok, let's look at this next week. I would be more happy too if everything is perfect and up to standard. Please let me know if any changes are needed and have a nice time.

JaffaKetchup · 2022-06-29T14:43:36Z

@aytunch
I'm done with exams now, luckily for the recent high priority issue.
After I've dealt with that, I'll go through this. Should be quicker than next week, but I can't promise.

ibrierley · 2022-06-29T15:03:01Z

Took a quick peak at leaflet out of interest...so I think part of the confusion is that we don't behave the same with Anchors to them.

In my brain, I'm along with leaflet.

Lets say the Marker width/height is 300px/100px, the "point" of the Anchor would be at 75,100 (referenced top/left like they say in docs), however to have the same effect in flutter_map we need to set it to be 225,0.

Why we don't follow Leaflet in this case I don't understand.... :D.

JaffaKetchup · 2022-06-29T15:07:42Z

Yeah I would agree. Using top left as (0, 0) definitely makes more sense. Whether we want to make as big of a breaking change as this is a different question.

ibrierley · 2022-06-29T15:15:52Z

Interesting, just found this....

#66

ibrierley · 2022-06-29T15:18:17Z

Actually, that may not be relevant thinking about it....

aytunch · 2022-06-29T15:19:44Z

Interesting, just found this....

#66

@ibrierley Nice find from 2018 :)
As @JaffaKetchup said, then the decision is between introducing a breaking change or not. I don't think the devs would like changing Anchor positions too much. Is there any non-breaking solution for having a (0,0) top-left Anchor coordinate system?

aytunch · 2022-06-29T15:20:39Z

Actually, that may not be relevant thinking about it....

I think it is related. But the author only cared about the vertical convention not the horizontal

ibrierley · 2022-06-29T15:28:59Z

Yeah, and don't worry, I won't be suggesting any breaking changes at all short term (one could conceivable consider your fix very slightly changing in behaviour if they've coded some hack workaround, so we probably need a minor release, but I doubt anyone ever has hacked around it).

My main concern with this type of stuff is making sure any "oddness" gets documented in the code (and main docs for usage), so there's as easy logic as possible (partly as people often reuse code later on), that way next time, any devs don't spend too long on it iyswim.

aytunch · 2022-06-29T15:35:28Z

We should definitely state in documentation https://docs.fleaflet.dev/usage/layers/marker-layer#markers-markers that;

Anchor.left corresponds to the logical distance between the Anchor point from the right side of the Marker Rect
Anchor.top corresponds to the logical distance between the Anchor point from the bottom side of the Marker Rect

JaffaKetchup · 2022-07-03T12:04:40Z

I'll try to have a look at this today if possible.

aytunch · 2022-07-07T19:20:29Z

Hi @JaffaKetchup @ibrierley how are you guys?

I was using my PR in the production app and everything was fine for disappearing markers.
However, I just integrated map clustering plugin, and it forces me to use the main branch of flutter_map
So, hopefully I am not being too pushy, but can I ask any of you to take a look at this PR?
I even have hopes that it might solve some disappearing issues of the spidered clusters because of the same reasons.

Meanwhile, if you know a way to use this PR in dependencies along with clustering plugin, it would help me out a lot too.

video from the attached cluster issue:

spider_disappear.mov

JaffaKetchup

Sorry for the slow reply @aytunch, totally my fault, keep saying I'll do it, then I never do.
By just looking at the code and the videos, I can tell it will work, and as it's a simple change, I don't feel the need to test it - we can always revert if absolutely necessary.

Note that it may take a while for the version to become available on pub.dev, unfortunately. This is because I need to write the CHANGELOG for the version, which needs approval and merging.

JaffaKetchup · 2022-07-07T19:26:00Z

In the meantime, you may try using 'dependency_overrides'?

aytunch · 2022-07-07T19:35:13Z

@JaffaKetchup there is no fault. We are very thankful to both of you for developing and maintaining this awesome package and your personal plugins as well.

As for the changelog, maybe something along these lines might be enough
Logic to decide if a Marker is off-screen now takes the Anchor point in to account. (Previously it assumed symmetrical anchors)

which needs approval and merging.

Who does the approval and merging btw?

JaffaKetchup · 2022-07-07T19:38:32Z

Thanks :)

CHANGELOG should be simple enough, but there are some other changes to put in there from previous commits.

If I submit the CHANGELOG PR - which is annoying at the moment, as I already have a fork for #1294, so I'll have to get involved with Git (~~help me~~) - then @ibrierley will have to approve it. Or vice versa. We can't merge our own PRs.

aytunch · 2022-07-07T19:43:56Z

I see. Good luck with #1294
If there is anything I can help with, please let me know guys.
On another note, my next milestone is integrating flutter_map_tile_caching so expect some questions and issues there too :D
MapBox is very expensive and we don't have an investor yet. Your plugin will come to the rescue ;)

JaffaKetchup · 2022-07-07T19:45:30Z

Hey, always glad to have a new user! Happy to answer questions.

Heads up, please use the v5 branch, or pub.dev prereleases. There are lots of breaking changes to get from v4 to v5, so there's no point implementing it.

JaffaKetchup · 2022-07-07T20:44:58Z

@aytunch Just thought I should quickly mention, I just broke the v5 branch (see JaffaKetchup/flutter_map_tile_caching#44 (comment)), so use the pub.dev pre-releases which still work. Also avoid using background downloading until the next pre-release - I'm changing it a lot.

Proposed fix for unsymmetrical anchored markers disappearing

85ad5ed

changed width&height to the new values in Positioned

aa04932

aytunch added 5 commits June 29, 2022 01:32

added prints

a7611ee

Update marker_layer.dart

24299ed

Update marker_layer.dart

64ce554

Update marker_layer.dart

26a4551

fixed calculations after debug and tested

7c848c3

changed var to final

90c91be

ibrierley reviewed Jun 29, 2022

View reviewed changes

fixed typos

9967901

aytunch requested a review from ibrierley June 29, 2022 11:09

ibrierley reviewed Jun 29, 2022

View reviewed changes

changed naming convention for sw and ne to make more sense

52b2352

added comment for the coordinate system confusion of Anchors

4bbf1cb

aytunch mentioned this pull request Jul 6, 2022

Inconsistent exploding behaviour within clusters and more lpongetti/flutter_map_marker_cluster#123

Closed

JaffaKetchup approved these changes Jul 7, 2022

View reviewed changes

JaffaKetchup merged commit 9de1b0f into fleaflet:master Jul 7, 2022

aytunch mentioned this pull request Aug 11, 2022

Null check bug fix lpongetti/flutter_map_marker_cluster#132

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proposed fix for unsymmetrical anchored markers disappearing #1291

Proposed fix for unsymmetrical anchored markers disappearing #1291

aytunch commented Jun 28, 2022 •

edited by JaffaKetchup

Loading

aytunch commented Jun 28, 2022

aytunch commented Jun 28, 2022

aytunch commented Jun 29, 2022 •

edited

Loading

ibrierley left a comment

aytunch commented Jun 29, 2022

ibrierley left a comment

aytunch commented Jun 29, 2022 •

edited

Loading

aytunch commented Jun 29, 2022 •

edited

Loading

ibrierley commented Jun 29, 2022 •

edited

Loading

aytunch commented Jun 29, 2022

JaffaKetchup commented Jun 29, 2022

ibrierley commented Jun 29, 2022

JaffaKetchup commented Jun 29, 2022

ibrierley commented Jun 29, 2022

ibrierley commented Jun 29, 2022

aytunch commented Jun 29, 2022 •

edited

Loading

aytunch commented Jun 29, 2022 •

edited

Loading

ibrierley commented Jun 29, 2022

aytunch commented Jun 29, 2022 •

edited

Loading

JaffaKetchup commented Jul 3, 2022

aytunch commented Jul 7, 2022 •

edited

Loading

JaffaKetchup left a comment

JaffaKetchup commented Jul 7, 2022

aytunch commented Jul 7, 2022 •

edited

Loading

JaffaKetchup commented Jul 7, 2022

aytunch commented Jul 7, 2022 •

edited

Loading

JaffaKetchup commented Jul 7, 2022

JaffaKetchup commented Jul 7, 2022

Proposed fix for unsymmetrical anchored markers disappearing #1291

Proposed fix for unsymmetrical anchored markers disappearing #1291

Conversation

aytunch commented Jun 28, 2022 • edited by JaffaKetchup Loading

aytunch commented Jun 28, 2022

aytunch commented Jun 28, 2022

aytunch commented Jun 29, 2022 • edited Loading

ibrierley left a comment

Choose a reason for hiding this comment

aytunch commented Jun 29, 2022

ibrierley left a comment

Choose a reason for hiding this comment

aytunch commented Jun 29, 2022 • edited Loading

aytunch commented Jun 29, 2022 • edited Loading

ibrierley commented Jun 29, 2022 • edited Loading

aytunch commented Jun 29, 2022

JaffaKetchup commented Jun 29, 2022

ibrierley commented Jun 29, 2022

JaffaKetchup commented Jun 29, 2022

ibrierley commented Jun 29, 2022

ibrierley commented Jun 29, 2022

aytunch commented Jun 29, 2022 • edited Loading

aytunch commented Jun 29, 2022 • edited Loading

ibrierley commented Jun 29, 2022

aytunch commented Jun 29, 2022 • edited Loading

JaffaKetchup commented Jul 3, 2022

aytunch commented Jul 7, 2022 • edited Loading

JaffaKetchup left a comment

Choose a reason for hiding this comment

JaffaKetchup commented Jul 7, 2022

aytunch commented Jul 7, 2022 • edited Loading

JaffaKetchup commented Jul 7, 2022

aytunch commented Jul 7, 2022 • edited Loading

JaffaKetchup commented Jul 7, 2022

JaffaKetchup commented Jul 7, 2022

aytunch commented Jun 28, 2022 •

edited by JaffaKetchup

Loading

aytunch commented Jun 29, 2022 •

edited

Loading

aytunch commented Jun 29, 2022 •

edited

Loading

aytunch commented Jun 29, 2022 •

edited

Loading

ibrierley commented Jun 29, 2022 •

edited

Loading

aytunch commented Jun 29, 2022 •

edited

Loading

aytunch commented Jun 29, 2022 •

edited

Loading

aytunch commented Jun 29, 2022 •

edited

Loading

aytunch commented Jul 7, 2022 •

edited

Loading

aytunch commented Jul 7, 2022 •

edited

Loading

aytunch commented Jul 7, 2022 •

edited

Loading