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

Geometry.Point Equals returns true when Lat/Long on both points are null. #149

Closed
morehavoc opened this issue Mar 15, 2023 · 4 comments · Fixed by #220
Closed

Geometry.Point Equals returns true when Lat/Long on both points are null. #149

morehavoc opened this issue Mar 15, 2023 · 4 comments · Fixed by #220
Assignees

Comments

@morehavoc
Copy link
Member

morehavoc commented Mar 15, 2023

TL;DR

I think we need to do four things:

  1. Remove the check for duplicate geometries, and add whatever the dev gives us
  2. Figure out the lat/long issue and fix Equas to work correctly. This might be as simple as fixing it to be && instead of || between the lat/long and x/y/z comparisons. But also, what about M values?
  3. In a Graphic, make equality only work on id (We should discuss this, perhaps it should just include more, for example, a Graphic could be the same geometry and same attributes but a different symbol, those would be two different graphics.)
  4. Check the other geometries for similar or other comparison things

And now the rest of the story

I discovered this while adding points to graphics layers. It turns out that GeoBlazor checks to see if the graphic has already been added... I don't think we should do this check. It seems strange to add the same graphic multiple times.. but I can see it as a workflow. Maybe I need to throw them on the map and then be able to do a count when the user clicks? So.. I think we should remove that, or if we need it, only check the ID. It's also a little expensive if I have lots of graphics!

But back to the problem at hand. When comparing two graphics, it checks the Geometry and these two points are coming back as true (Equal).

Point 1:
image

Point 2:
image

These two points are not the same. However, they do both have the same Lat/Long values... null.

If I understand this function correctly:

public bool Equals(Point? other)
    {
        if (ReferenceEquals(null, other)) return false;
        if (ReferenceEquals(this, other)) return true;

        return (Latitude.Equals(other.Latitude) && Longitude.Equals(other.Longitude)) ||
            (X.Equals(other.X) && Y.Equals(other.Y) && Z.Equals(other.Z));
    }

Then Latitude.Equals(other.Latitude) when both Latitude's are null will be true. This proves to be true in an immediate window as well:
image

The docs for Lat and Long properties in Javascript say:

The latitude of the point. If the spatial reference is Web Mercator, the latitude will be given in WGS84. In any geographic spatial reference, the latitude will equal the y coordinate. In all other cases the latitude will be null.

In my case, I am using geometries that I have done things to, projected, densified, etc. and the Lat/long values are not populated. So, we need to sort out when lat/long should participate in the equality test, if ever.

@TimPurdum
Copy link
Collaborator

@morehavoc once again the rub comes down to the razor component rendering support. Without an equality check, it is possible for the rendering cycle to continue adding the same graphic to the layer or view, since we are manually registering everything. It is possible that this is no longer an issue given other changes, but that would be my main concern to test, that removing equality checks does not create many duplicates in markup-rendered graphics.

As far as just updating the equality check, I'm 100% for this. Would it work to just do && between Lat/Long and X/Y/Z? If either set is null, it would still check the other set.

@TimPurdum TimPurdum self-assigned this Aug 1, 2023
@TimPurdum
Copy link
Collaborator

@morehavoc I'm revisiting this open gissue. It doesn't seem to be impacting users much, however, I would like to resolve it. Based on the doc link you shared above, I think we could do equality checks only on X/Y, and skip Lat/Long, since they are either null, or the same as X/Y. Does that make sense?

@morehavoc
Copy link
Member Author

@TimPurdum For point geometries, I think we might have to check all of them: Lat, Long, X, Y, Z, M. I think, based on your descriptions of how we have to keep track of things between renders, that we could have points that are Lat, Long, but no other properties, so I think we have to check all 6 values to ensure they are the same (they can be null, as long as they are null in both points).

For other geometries, we should check each point in the list to verify that each one is the same, and that each point needs to be compared on all of its properties (which I think are X, Y, Z, M at most).

@TimPurdum
Copy link
Collaborator

@morehavoc I've been doing more digging, and you were correct. I fixed the re-rendering issue that is at the root of GeoBlazor awhile ago, and I am in the process of testing removing all the equality checks.

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.

2 participants