Improvements to cameras

[almarklein]

I already made some changes to the cameras in #612, and I have some further ideas for improvements.

1) Bounds The Visual class has a bounds method which subclasses must implement to expose their "coverage area". We should use these to automatically set the camera to show all the content in the scene. The SubScene object hasa get_scene_bounds() method that calculates the total bounds of the scene. The camera uses this when camera.reset() is called. A reset takes place the first time that a camera is associated with a viewbox. This means that if you first put visuals in the scene, and then assign a camera, things Just Work.

2) Zoom direction We should decide whether scrolling up or down zooms in. Similarly zooming with RMB. These two should also be consistent.

3) scale_factor on all cameras. In #612 I gave the BaseCamera a scale_factor property, which is initially determined from the range (in set_range), but can subsequently be modified to modify the scale. For the turntable camera this directly corresponds to zooming. For the Fly camera this determines the speed of the vessel. I did not apply this to the PanZoomCamera yet, but it should also directly correspond to zooming. Actually, I think we could bind the scroll wheel to this attribute in the base camara class.

4) center on all cameras. All camera's have some aspect of the center. The center of rotation for turntable, the center of the visible area for panzoom, and for the fly camera the position of the vessel. The center can be determined in set_range()

5) fov on all camera. Some visuals (like Volume), need this info, is convenient that its always there. Correction: Only PerspectiveCamera and derived classes need this parameter.

6) Multiple cameras / multiple scenes. Would be good to make it easy to have multiple cameras in one scene, and also to have one camera be used by multiple scenes. Both now works per #612.

7) aspect ratio of the scene. The scene or viewbox should have an aspect_ratio property, which is a 3-element tuple that defines the scaling of each dimension. Negative values indicate flipping of that dimension. Camera's should deal with this, and e.g. plots should be zoomable in two dimensions (unless this is disabled). I got this working pretty well in visvis (and I remember it was painful to implement :( )

Checklist (for what we agree on):

• [x] bounds / limts
• [x] zoom scroll direction
• [x] scale factor
• [x] center
• [x] fov
• [x] Multiple cameras / multiple scenes
• [x] aspect ratio

[almarklein]

@lcampagn and I had a gitter chat about this: here are some conclusions (Luke, please correct me if I am wrong or missed something):

• The camera classes need to be in the scene namespace.
• Get rid of set_camera; people can use viewbox.camera = scene.SomeCamera().
• People can use convenience viewbox.camera = 'turntable' This will select a matching camera if it is already present in the scene, or otherwise create the corresponding camera with default arguments.
• We discussed some points about multiple cameras in one scene, and multiple scenes associated with one camera. We don't expect much problems here.
• fov property should only be on cameras for which it makes sense, otherwise it will confuse users.

[almarklein]

We also discussed the scale factor and bounds. An alternative to camera.set_range(), might be that the viewbox provides a means to get the total bounds for all objects in the scene, and that the camera queries this information when it wants to reset.

[almarklein]

All features are implement in #612, see example/scene/one_scene_four_cameras.py. Only the data aspect needs some work to get really right.

[campagnola]

1) I like set_range because 'bounds' and 'limits' somewhat imply a region that the camera should be confined to (the user may not move the camera outside this range). For automatically viewing certain visuals there is already PanZoomCamera.auto_zoom(visual), which is used by the plot functions. I might prefer camera.auto_range() to match set_range, though. Then viewbox.auto_range() would just be a shortcut for camera.auto_range(viewbox.scene).

2) Wheel up should zoom in; that is the standard I have seen in most applications. It should also be easily configurable (but we can worry about that later). If there are platforms with different expectations (OSX, I'm looking at you) then we could also make the default behavior platform-dependent.

Will have a look at the new example..

[almarklein]

Maybe I should have updated this post: set_range and auto_zoom are both gone. Instead, camera.reset() gets the bounds of the scene and sets the camera settings appropriately. Reset is called automatically the first time it is associated with a viewbox.

I always get confused what "scrolling up" means. But see the example and let me know whether it does what you expect :)

[campagnola]

Here's my current list of comments:

Non-issues:

• I like having BaseCamera.fov (=0 for PanZoomCamera)
• I think it probably makes sense for PanZoomCamera to inherit from PerspectiveCamera, but I'm not quite decided on this.

Major issues:

• Not fond of reset(): it's not obvious what "resetting" a camera should do. I would prefer to be explicit. Possiblities:

  camera.auto_view(scene) 
  camera.set_range(visual) 
  camera.zoom_to(scene)
  camera.show(visual)

• I am not convinced it is worthwhile to have one camera in multiple views. This greatly complicates cameras while adding little benefit--it should be quite straightforward to synchronize two cameras.
  • BaseCamera needs save_state() and restore_state() so we could easily transfer state from one camera to another (as well as providing a generally useful feature)

• Not fond of scale_factor.

  • What does scale_factor mean for PanZoomCamera, given that its axes scale independently?
  • Seems difficult for the user to predict what will be visible? (as opposed to setting camera state explicitly)
  • scale_factor seems to have two uses: setting the initial zoom level and zooming interactively. For the first use, the user will probably prefer something like camera.view(visual) or camera.set_range(limits). For the second use, I prefer something like camera.zoom(factor), which uses a relative scale factor rather than an arbitrary absolute factor:

    def BaseCamera.zoom(self, amount, center=None):
        """ Scale the view isotropically by *amount* relative to the given center point. 
  
        1.0 leaves the view unchanged; 2.0 makes items appear twice as large
        Subclasses may implement extra arguments such as a center point to zoom around,
        whether to zoom by distance or fov, 
        """

Minor issues:

• The need for _ref_viewbox suggests that perhaps we should use viewbox.auto_view(visual) instead?
• is _center_loc relative to the local coord system? That's something we need to figure out.. Should cameras be moved like camera.pos = or camera.transform.translate = ?
• Zooming is much less sensitive in PanZoom camera
• examples/basics/scene/grid.py is broken
• Need some equivalent to Camera.auto_zoom

[larsoner]

Thanks for bringing this up -- it looks like a couple of PRs (#611 and #612) depend on sorting out these issues, so it would be good to work them out.

-

Not fond of reset(): it's not obvious what "resetting" a camera should do. I would prefer to be explicit. Possiblities:

I like "auto_view", since it automatically sets the view to encapsulate the whole scene.

I agree it is more explicit than "reset". It could take a scene or a single visual as an argument, although starting with a scene seems like a good / most widely used starting point.

-

I am not convinced it is worthwhile to have one camera in multiple views.

I also don't have any use cases in mind for two cameras.

I agree something like a "get_state" and "set_state" that return and take some object / state to set up the cameras would work well. To start you might have to always do "camera2.set_state(camera1.get_state())" at the end of each draw call. If users clamor for a way of automating this relationship, we can think about doing a publisher/subscriber framework or something. But if it cleans up the code to remove it, I'm happy to assume YAGNI or let users manually do it to start with.

-

Not fond of scale_factor.

I agree that your idea of using "zoom" as a relative operation is intuitive. If scale_factor is necessary for some application or Almar is really attached to it, maybe it could be rebranded as "zoom_factor" or something. :)

I'll let Almar comment on the minor issues since they seem more directly relevant to his / your implementations of cameras.

Cheers, Eric

[almarklein]

I like having BaseCamera.fov (=0 for PanZoomCamera)

Funny, I thought you were the one being against that at some point :)

Sure, we can rename reset()

I'll have a look how the code will look like when we allow just one viewbox per camera. I like your idea of syncing cameras instead. It also feels less awkward (as opposed to having a camera that is in two scenes at the same time).

The scale factor is sort of a generalization of a zoom factor. I did not call it zoom because the exact value and interpretation depends on the camera. E.g. in the fly camera it is used to establish a notion of the scale of the scene. In case of of per-axis zooming (as the panzoom camera can do), the scale factor applies to the first axis, and the aspect_ratio determines how the other axises (axi?) relate to it.

Having a zoom function with a relative value makes a lot of sense, especially since the specific value of scale_factor might seem a bit arbitrary to the user. I also do not see a user explicitly specify the scale_factor, but he/she might store and reset it, or set it from another camera.

is _center_loc relative to the local coord system? That's something we need to figure out.. Should cameras be moved like camera.pos = or camera.transform.translate = ?

It is relative to the parent coord system (in most cases the root of the subscene). Cameras adjust their own transform, so users should not use camera.transform directly.

Zooming is much less sensitive in PanZoom camera

Indeed, I played with these. Zooming of panzoom and Turntable should now be consistent. I think it was previously rather sensitive, but I can turn it up a bit if you want.

I'll have a go at this when I can find some time next week.

[almarklein]

Ok, I fixed most issues mentioned by Luke:

• fov is property of the BaseCamera
• camera.reset() -> set_range()
• camera has get_state() and set_state() methods
• camera has link() method to link cameras in different scenes
• camera is now associated with 0 or 1 viewboxes, not more. Simpler code!
• grid.py fixed

Remaining points:

• I am not sure about having PanZoom subclass Perspective. In my view Perspective is a "3D" camera. If PanZoom would inherit from it, we might as well do Perspective == BaseCamera.
• I did not do a Camera.zoom() yet. PanZoomCamera already has a method zoom(center, scale_factor). Should we rename this one? Also I realized that camera.zoom(x) would simply do camera.scale_factor *= x. Do we need a function for that?
• The sensitivity of the zooming can easily be adjusted with a constant in the top of the module. Maybe we should fiddle with that in Paris.

[larsoner]

This sounds reasonable to me, and I don't have any strong opinions on the last few points. Are these changes part of #612, or are they on a different branch (volume2)?

[almarklein]

The are in #612. We still need a decision on camera.zoom().

[almarklein]

@lcampagn happy?

[larsoner]

FWIW I don't mind camera.scale_factor *= x, actually, so I'm okay with not having .zoom() now.

[campagnola]

Haven't had time to look at the camera changes yet, but I still see some issues in the examples (grid, viewbox, isosurface).

[dirtyfish]

view.set_camera('turntable', mode='perspective', up='z', distance=2,azimuth=30., elevation=65.) would now be?