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

Fixes bug where labels moved after placing them #3194

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

misaugstad
Copy link
Member

Fixes #1764
Further work towards #2485

This fixes a bug that cause labels to move slightly after placing them.

The issue was that there was a function where we were rounding the heading at pitch to an integer before calculating where it should be placed. Actually worse than that: we were just truncating the number to an integer using parseInt(). And since we restrict the pitch between 0 and -35, we were always treating the pitch like it was higher than it actually was (-12.99 truncated to -12), which is why labels always moved upwards. This is also more pronounced at higher zoom levels because of the smaller field of view; the same change in pitch had a bigger effect.

This also propagates to the sv_image_x/y values that we use for CV. These are changes by less than 100 pixels for the most part, given that it's a change in pitch/heading of less than 1 degree. But those small mistakes matter a lot!

This did not have an effect on how we saw labels in Validate, Gallery, or the Admin page. Which is good... But we also saw plenty of cases on Validate where users had labeled well above the actual issue... And I don't believe that my updates address that at all.

Our team has also noted other issues with the y position in the full GSV image that has something to do with the photographer pitch, and issues when on hills. But it's possible that our results there will be more conclusive once this confounding source of error is removed!

And finally, I have not made the changes retroactively in the database yet, but we do have all the info that we need to fix these!

Before/After screenshots (if applicable)

Below I try to label directly in the center of the shadow X. My label gets moved up and to the left. This is because I set the heading to 325.9 and pitch to -17.9. They are truncated to 325 and 17, moving the label to the left and up.
Screenshot from 2023-03-30 18-00-18

I then made the update to the code, and the labels are even rerendered correctly on page reload! When I try to add a label directly on top of it, it works correctly as well!
Screenshot from 2023-03-30 18-00-46

Things to check before submitting the PR
  • I've written a descriptive PR title.
  • I've added/updated comments for large or confusing blocks of code.
  • I've included before/after screenshots above.

@misaugstad misaugstad self-assigned this Mar 31, 2023
@misaugstad misaugstad merged commit 6b3921f into develop Mar 31, 2023
@misaugstad misaugstad deleted the 2485-fix-sv-image-y branch March 31, 2023 01:23
@jonfroehlich
Copy link
Member

Love that you found the perfect image to test this with an "X" marks-the-spot shadow.

@misaugstad misaugstad mentioned this pull request Apr 11, 2023
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.

Labels move after placement
2 participants