Improve viewer camera #236

mramato · 2012-09-12T16:36:06Z

The Viewer widget has a simple tracking capability which tracks an object when you right-click on it. This only worked in 3D mode and has been recently broken. I created a new utility object, DynamicObjectView which makes it easy to track an object across scene modes.

Right-click on an object in viewer.
When you switch modes, you'll stay locked onto the thing you are tracking (though transitions aren't supported yet).
If you change the view in one mode, it will be reflected to have a similar view in the new mode.
Added a viewFrom property to CZML which is an east-north-up Cartesian that gives the client a recommended camera position when zooming to the object.
@bagnell fixed a bunch of issues that were uncovered while I was working in this branch.

This whole process has also exposed many issues with our current camera API that will be addressed in future pull requests, DynamicObjectView is a little overly complicated because of this and unit tests for the new object is light for now.

…cross all scene modes.

…work on DynamicObjectView, though its currently a mess.

… 3D.

1. Make lookAt query parameter work again in Viewer 2. Fix several crashes in DynamicObjectView 3. Fix crash in BoundingRectangle and make BoundingSphere work the same way.

Conflicts: Source/Core/BoundingRectangle.js Specs/Core/BoundingRectangleSpec.js Specs/Core/BoundingSphereSpec.js

…template, since all 3 are required, a template doesn't make sense.

Conflicts: Source/Widgets/Dojo/CesiumViewerWidget.js

…sform after translating.

pjcozzi · 2012-09-13T19:57:33Z

I'm able to right-click anywhere on the globe with pretty strange results.

pjcozzi · 2012-09-13T19:58:43Z

I can zoom in/out when viewing the facility, but not when viewing the satellites.

pjcozzi · 2012-09-13T20:02:05Z

The Home View button restores a 2D view to 3D. I believe it always did that. Is that the right behavior? I would expect it to change views, but not scene modes.

pjcozzi · 2012-09-13T20:06:20Z

I can zoom in/out when viewing the facility, but not when viewing the satellites.

This is in simple.czml. It's odd though - sometimes I can zoom, but usually I can't.

pjcozzi · 2012-09-13T20:11:29Z

Added a viewFrom property to CZML which is an east-north-up Cartesian that gives the client a recommended camera position when zooming to the object.

I'm not sure about this property. What is the long-term plan? East-north-up is a decent default, but it is not going to work in many air and space situations, when we really want the object's body frame. Is there a longer-term plan to be able to overview east-north-up?

pjcozzi · 2012-09-13T20:14:09Z

Source/Scene/Camera2DController.js

@@ -63,6 +63,24 @@ define([
            throw new DeveloperError('projection is required.');
        }

+        /**
+         * If true, allows the user to pan around the map.  If false, the camera stays locked at the current position.


I'm confused - is this enableTranslate or enablePan. Panning is more than translating, right?

Two me they mean the same thing (in 2D) but I could be wrong. I can change the wording but can you explain the difference to me?

Oh right, this is only on the 2D controller. Translate and pan are the same here.

pjcozzi · 2012-09-13T20:19:15Z

@shunter want to look at the DynamicScene changes?

pjcozzi · 2012-09-13T20:21:21Z

Source/Scene/Camera.js

-            up = arguments[2];
-        } else {
-            return;
+    Camera.prototype.lookAt = function(eye, target, up) {


Good change. I looked at this function pretty recently, and thought we should get right of using arguments. This is very early code.

pjcozzi · 2012-09-13T20:25:48Z

Not that I'm the right person to ask, but the scene-level camera changes are OK with me. There's a bunch of allocations in new code. Is the plan to remove them now or later?

mramato · 2012-09-13T20:54:48Z

I plan on replacing right-click with a single left-click pop-up. This pop-up will ultimately be the balloon for the object as well as other context-sensitive actions. I'm trying to avoid double-clicks so that things can work on mobile as well. I left it the way it was for now because I didn't want to worry about UI and camera behavior at the same time.

mramato · 2012-09-13T20:55:16Z

Can you elaborate on "strange results" One right-click should track and the other should stop tracking.

pjcozzi · 2012-09-13T20:59:19Z

Can you elaborate on "strange results" One right-click should track and the other should stop tracking.

When it stops tracking, the user is left in the object's old frame - doesn't seem useful.

pjcozzi · 2012-09-13T21:02:09Z

I plan on replacing right-click with a single left-click pop-up. This pop-up will ultimately be the balloon for the object as well as other context-sensitive actions. I'm trying to avoid double-clicks so that things can work on mobile as well. I left it the way it was for now because I didn't want to worry about UI and camera behavior at the same time.

Zoom is so common that it is going to require a shortcut like double-click or double-tap. A context menu will be nice for listing tasks, but we also need to optimize for the common case. Right click is like a secret handshake; the only people that know to do it are the people that were told. I'm pretty sure @kring just hacked it in for a demo.

mramato · 2012-09-13T21:03:00Z

Zooming in and out for satellites works for me, of course if I'm really close it takes a while to zoom out.

mramato · 2012-09-13T21:03:21Z

Good call on the home button, I've been meaning to change it. I'll update it now.

mramato · 2012-09-13T21:05:11Z

The viewFrom is a temporary property for now, a stop gap to enable some demos until we figure out how we want everything to work "for realz"

pjcozzi · 2012-09-13T21:22:12Z

Zooming in and out for satellites works for me, of course if I'm really close it takes a while to zoom out.

Hmm. Perhaps it was the speed because I can't reproduce it now, but it really seemed like it was locked, not just slow to start. I'll let you know if I come across it again.

mramato · 2012-09-14T03:08:16Z

The home button now maintains the current scene mode. The code to do it is more complicated than it needs to be, but one of the main points of this exercise was to expose these kinds of issues. It will be simplified when we better abstract the camera API.
Change zoom-to from right-click to left-double-click. I also changed the behavior so that once you are tracking something, the way to un-track is to either double-click on something else or hit the home button. Double clicking on empty space no longer stops tracking. I think this is much more usable.
You had made a comment on memory allocations; the camera needs a huge amount of cleanup and API overhaul, so I'm not concerned about them yet, they'll get cleaned up on a file-by-file basis as we refactor the API.

mramato · 2012-09-14T03:09:03Z

Assuming I didn't miss anything, this is ready for another round of review.

pjcozzi · 2012-09-14T11:43:10Z

These changes are good with me.

@shunter want to look at the DynamicScene code changes and merge?

shunter · 2012-09-14T13:59:31Z

Source/DynamicScene/DynamicPointVisualizer.js

@@ -226,7 +226,7 @@ define([
        billboard.setShow(true);

        position = positionProperty.getValueCartesian(time, position);
-        if (position !== 'undefined') {
+        if (typeof position !== 'undefined') {


I did a grep and found a similar mistake in DynamicBillboardVisualizer. Want to fix that too?

Done. I've been meaning to write a general regex to find these, did you have one, or did you just do a simple search and happen to find it?

Not a single regex, but: grep -r "'undefined'" Source/ | grep -v typeof

I should mention that there's still a similar mistake in CentralBody, but all that's changing anyway soon so it's not worth bothering with.

shunter · 2012-09-14T14:12:58Z

Looks good.

Improve viewer camera

mramato and others added 30 commits August 20, 2012 22:31

Start of utility class for tracking a DynamicObject with the camera a…

db74477

…cross all scene modes.

Get tracking in both 2D and 3D working.

acdea66

Merge branch 'master' into viewerCamera

e9cfbd2

Initial support for new viewFrom property.

fbcf9b5

Add a helper function, setCameraPosition, to the 2D controller. More …

dd91253

…work on DynamicObjectView, though its currently a mess.

Merge branch 'master' into viewerCamera

def7c5d

Merge remote-tracking branch 'origin/mtl-shader-gen' into viewerCamera

551eadf

Maintaining view state across scene modes now works nicely for 2D and…

b3c5d45

… 3D.

Several fixes

b7a4bda

1. Make lookAt query parameter work again in Viewer 2. Fix several crashes in DynamicObjectView 3. Fix crash in BoundingRectangle and make BoundingSphere work the same way.

Merge branch 'affectedByLighting' into viewerCamera

08b6f92

Merge branch 'cullingFixes' into viewerCamera

c6b22a6

Conflicts: Source/Core/BoundingRectangle.js Specs/Core/BoundingRectangleSpec.js Specs/Core/BoundingSphereSpec.js

Merge branch 'master' into viewerCamera

63cd2dd

Merge remote-tracking branch 'origin/mtl-shader-gen' into viewerCamera

8046093

Merge branch 'master' into viewerCamera

bef5b0c

Merge branch 'master' into viewerCamera

57ed3ab

Merge branch 'master' into viewerCamera

15afd2d

Change Camera.lookAt to use parameters instead both parameters and a …

14c4eaf

…template, since all 3 are required, a template doesn't make sense.

Merge branch 'master' into viewerCamera

6bc8f85

Also toggle atmosphere whenever lighting is toggled.

1e801fe

Merge branch 'master' into viewerCamera

6360106

Merge branch 'ellipsoid' into viewerCamera

361a159

Merge branch 'transforms' into viewerCamera

3507456

Cleanup the code and add doc, still need columbus view mouse handling.

aa17f31

Merge branch 'master' into viewerCamera

3bc6c89

Merge branch 'boundingSphereFromPoints' into viewerCamera

b01a1cd

Merge branch 'extent' into viewerCamera

3941073

Conflicts: Source/Widgets/Dojo/CesiumViewerWidget.js

Modify CameraColumbusViewController so it doesn't set the camera tran…

c7e95ef

…sform after translating.

Fix CameraColumbusViewController zoom.

9bd74c1

DynamicObjectView now works in all modes.

5d46b54

Fix mouse handling after cancelled columbus view morph.

6803de4

pjcozzi reviewed Sep 13, 2012
View reviewed changes

mramato added 3 commits September 13, 2012 17:23

Merge branch 'master' into viewerCamera

59a88d7

Viewer "view home" button now stays in the current scene mode.

16985b9

Change zoom-to from right-click to left-double-click.

dc67af6

shunter reviewed Sep 14, 2012
View reviewed changes

mramato and others added 2 commits September 14, 2012 10:06

Fix missing typeof

8e80521

Whitespace and comment tweaks.

049f5b6

shunter added a commit that referenced this pull request Sep 14, 2012

Merge pull request #236 from AnalyticalGraphicsInc/viewerCamera

da591fa

Improve viewer camera

shunter merged commit da591fa into master Sep 14, 2012

snyk-bot mentioned this pull request May 31, 2023

[Snyk] Upgrade magic-string from 0.25.9 to 0.30.0 YunSumin/cesium#4

Open

Improve viewer camera #236

Improve viewer camera #236

Uh oh!

Conversation

mramato commented Sep 12, 2012

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

mramato commented Sep 13, 2012

Uh oh!

mramato commented Sep 13, 2012

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

mramato commented Sep 13, 2012

Uh oh!

mramato commented Sep 13, 2012

Uh oh!

mramato commented Sep 13, 2012

Uh oh!

pjcozzi commented Sep 13, 2012

Uh oh!

mramato commented Sep 14, 2012

Uh oh!

mramato commented Sep 14, 2012

Uh oh!

pjcozzi commented Sep 14, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shunter commented Sep 14, 2012

Uh oh!

Uh oh!