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

Keep camera position during a morph when duration is 0.0 #3521

Merged
merged 29 commits into from
Mar 11, 2016
Merged

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Feb 4, 2016

For #1060.

Fixes #2065. May also fix #1518 and #1845, though I couldn't reproduce #1845 in master either.
Might want to close #451 too since the morph view at the end is whatever the user set it to.

…er the last tiles that were rendered before the morph was started.
…era direction in the center of the screen when going from Columbus view to 2D.
@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 7, 2016

@bagnell @mramato let me know when the kinks are worked out, and I will take a quick look at the code.

@bagnell
Copy link
Contributor Author

bagnell commented Feb 12, 2016

@pjcozzi This is ready for another review. I decided for an incremental improvement until the other issues can be resolved.

If the duration is 0.0, the camera will maintain its position and orientation. In the Sandcastle, add viewer.sceneModePicker.viewModel.duration = 0.0;. Also, the issues mentioned above should still be fixed.

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 12, 2016

Code is OK with me. @mramato can you test and merge?

@mramato
Copy link
Contributor

mramato commented Feb 25, 2016

I do not agree with going back to the old fly out behavior. I also think it's confusing from a usability standpoint to have the camera stay where it is when duration is 0 vs fly out if it's non-zero. I'm super excited about the other morph fixes though (no more crashes!)

@bagnell can you please list what the remaining issues were that caused us to go back to the old behavior? The only thing I notice is jitter and tile seems.

In my opinion the graphical glitches during the morph are well worth the tradeoff to be able to morph in all cases without losing position. I'm not sure if others had time to see it in action, but git checkout 472d512cdf554fd1573dc4b42285b1743e66d741 in order to see for yourself. I'd be interested to hear what others' opinions are. If no one cases that we're still flying out, then I won't object to merging this; but it's such a common complain on the forum that I feel it's worth keeping the position around.

@hpinkos
Copy link
Contributor

hpinkos commented Feb 25, 2016

I do not agree with going back to the old fly out behavior

👍 People hate that they lose their position

But I don't think the morph in commit 472d512cdf554fd1573dc4b42285b1743e66d741 is quite right either. The morph between CV and 2D is good because even when you're zoomed in, you have a sense of what just happened. 3D to 2D, it's just weird to see the globe do a wiggle then stop. There's no context.
It's a little silly, but I think flying out, doing the morph, then flying back in is a better solution.
And I think it should tilt when you get into CV, otherwise you don't notice that the mode is any different from 2D until you use the middle mouse button.

Either way, I think this is a huge improvement. I haven't been able to switch between scene modes without the browser crashing in forever, so that's 100% worth merging in.

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 25, 2016

This incremental improvement is OK with me. The user this is for is just going to do a duration=0 "morph." We can revisit as time allows.

@bagnell
Copy link
Contributor Author

bagnell commented Mar 8, 2016

@mramato Those are the only two issues. For the second, it's not just cracks between tiles. There are whole tiles missing for certain views because the quadtree isn't updated during the morph.

@mramato @pjcozzi @hpinkos What are we doing with this PR?

@hpinkos
Copy link
Contributor

hpinkos commented Mar 8, 2016

I'm in favor of merging this in. I think there are some additional improvement that can be made further down the line, but this is much better than what we have now.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 8, 2016

+1 from me for merging this.

@mramato mramato changed the title Keep camera position during a morph. Keep camera position during a morph when duration is 0.0 Mar 11, 2016
@mramato
Copy link
Contributor

mramato commented Mar 11, 2016

I changed the title so when people stumble on this they don't get their hopes up. I just hope we eventually get back to making this work in all cases.

mramato added a commit that referenced this pull request Mar 11, 2016
Keep camera position during a morph when duration is 0.0
@mramato mramato merged commit 3e34ba2 into master Mar 11, 2016
@mramato mramato deleted the morph branch March 11, 2016 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants