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

Labels move after placement #1764

Closed
daotyl000 opened this issue Jun 28, 2019 · 14 comments · Fixed by #3194
Closed

Labels move after placement #1764

daotyl000 opened this issue Jun 28, 2019 · 14 comments · Fixed by #3194
Assignees

Comments

@daotyl000
Copy link

daotyl000 commented Jun 28, 2019

After placing and confirming a label, once I pan to shift my view the label would move to a different location. Sometimes slightly to still be ok but sometimes it will shift it very far away.

Link to a screen recording:
https://youtu.be/y0hdn6gk67I

@misaugstad
Copy link
Member

is this just happening on that pano, or are you seeing that elsewhere?

@misaugstad
Copy link
Member

also what browser are you using?

@daotyl000
Copy link
Author

I have seen it on several panos but it was never as bad as this time, I was on Safari.

@misaugstad
Copy link
Member

Interested to hear if others have this issue, particularly on Safari

@jonfroehlich
Copy link
Member

We need more investigation of this Issue... because if Sidewalk doesn't work well in Safari, we should let our users know (and encourage them to use an alternative browser).

So, for example, one heavy user of the Newberg deployment always had slightly misplaced labels, which made it hard to validate their work. I wonder if this was due to using Safari (@misaugstad, can you check what browser that user was using?)

@nch0w
Copy link

nch0w commented Jul 9, 2019

I'm not experiencing this issue on Safari, but I do notice that the label lags behind the panorama when panning.

@jonfroehlich
Copy link
Member

jonfroehlich commented Jul 9, 2019 via email

@nch0w
Copy link

nch0w commented Jul 9, 2019

It properly sticks to the tagged location.

@timvng
Copy link
Collaborator

timvng commented Jul 9, 2019

I don't have this issue on Safari. Although when I move the cursor onto the label, this happened.
Screen Shot 2019-07-09 at 12 08 46 PM
Now I can't delete the label anymore. I'm not sure if this has been mentioned before.

@jonfroehlich
Copy link
Member

jonfroehlich commented Jul 9, 2019 via email

@misaugstad
Copy link
Member

The rest of this thread above is a bit messy, but ultimately we are aware that labels will move right after you place them! This is worse when you are more zoomed in. The issue comes from the calculatePointPov and povToPixel3DOffset functions in UtilitiesPanomarker.js. These are pulled from the code for Panomarkers, and the math that they have there is pulled from this stackoverflow post. The person who wrote up the Panomarker code made a writeup for how the math works out here.

Unfortunately, there is clearly some error(s) in there! To be fair, part of the calculations include approximations. For example, Google documents the available field of view at different zoom levels, but they've noticed that these zoom levels are just not at all accurate in practice, so they've written up an approximation. But maybe things have changed since this code was written in 2014, and maybe we could improve upon it?

Here's the gist of the issue:

We start out with the pov (heading, pitch, zoom) of the GSV window and the pixel location on the canvas where the label was placed (canvasX, canvasY). We then use the calculatePointPov function mentioned above to figure, which figures out where the heading and pitch of GSV would have to be for the input label to be centered on the canvas.

Later on, we take that heading and pitch and use it to figure out where the label should now show up on the canvas after we've panned. To do that, we call the getCanvasCoordinate function (but all the math for that is in the povToPixel3DOffset function under the hood).

This all sounds fine! But one thing that should happen is that when you haven't panned at all, if you call calculatePointPov and pass the result into getCanvasCoordinate, you should get the same thing that you input to calculatePointPov. But unfortunately this is often not the case. Here is an example.

let currPov = {heading: 14.625, pitch: -15.625, zoom: 3};
let canvasX = 426;
let canvasY = 214;
let labelPov = util.panomarker.calculatePointPov(currPov, canvasX, canvasY, svl.CANVAS_WIDTH, svl.CANVAS_HEIGHT);
// result: {heading: 16.665, pitch: -13.966, zoom: 3}

// labelPov is the heading/pitch that GSV should be at to have the label centered on the canvas.
// Now let's use this and the exact same input POV to find canvasX/Y. It should be the same as our input X/Y...
util.panomarker.getCanvasCoordinate(labelPov, currPov, svl.CANVAS_WIDTH, svl.CANVAS_HEIGHT, svl.LABEL_ICON_RADIUS);
// result: {x: 411, y: 198}

In this example, we are off in the X direction by 15 pixels, and the Y direction by 16 pixels!

@jonfroehlich
Copy link
Member

This is great @misaugstad but I was expecting the kicker or reveal but you left us hanging. You state:

This all sounds fine! But one thing that should happen is that when you haven't panned at all, if you call calculatePointPov and pass the result into getCanvasCoordinate, you should get the same thing that you input to calculatePointPov. But unfortunately this is often not the case.

But then don't offer a suspected reason or possible solution... unless that's what you were trying to cover by "part of the calculations include approximations"

@misaugstad
Copy link
Member

This is great @misaugstad but I was expecting the kicker or reveal but you left us hanging

Sorry about that! That's partially because I needed to head off to the gym yesterday, and partially because I don't have a solution yet! My main goal of posting here was to actually document the problem with an easy to test example, with the issue narrowed down as much as possible.

I haven't actually dug into the math yet, or the writeup of the math that I linked to. It's possible that the math is all theoretically sound as long as the FOV approximations are correct... In that case, we could do something like we did for approximating label lat/lng (PR #2434). For that, I ran some linear regressions to find predictors of the distance from the panorama, giving us a decent estimate of lat/lng. Since we know what the output canvas x/y values should be (in the case where we haven't panned), we might be able to come up with a better approximation than we currently have.

But again, I haven't dug into the math enough yet :) The main update is that I've narrowed down the problem to a few lines of code, and cleaned up the surrounding code so that what we're looking at here makes a lot more sense and should be easier to debug when we try to do so again!

@jonfroehlich
Copy link
Member

Gotcha. Great progress on a thing that has long-plagued us! Thanks for the update @misaugstad!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants