v0.8 camera state and improved default rendering

[mattuntergassmair]

Camera state is now no longer part of RenderModel but instead an independent instance. As a result, the render functions become significantly simpler.

[mattuntergassmair]

it would be clearer to me if you added the following:

* a simple example usage for one or two of the camera

* testing
  I still have some confusions
  What is the difference between `set_camera!` and `update_camera!` why do we need both?

the previous implementation was doing that:

cam = MyCamera() 
update_camera!(rm, cam) 
render!(renderables, rm)

what is the equivalent now?

agreed, the naming is a bit confusing now, here are the differences:

• set_camera is literally a setter method for setting the state of the camera, i.e. fields like pos, zoom, rotation
• update_camera is taking a scene as input, does some computing, and eventually calls set_camera to change the camera state Do you have any ideas on how to make the naming more clear?

[MaximeBouton]

set_camera! is just internal then? The rendermodel still needs to know about the CameraState right? How is this done?

[MaximeBouton]

oh! I did not see that render! disappeared. This makes much more sense now. An example would definitely help.

Now the usage would be

update_camera!(cam, scene_or_whatever_is_needed)
render(renderables, camera-cam)

[mattuntergassmair]

set_camera! is just internal then? The rendermodel still needs to know about the CameraState right? How is this done?

Yes kind of, set_camera! acts on the CameraState and update_camera acts on the Camera object... agreed that we could probly just access the fields directly via camera.state.zoom etc and get rid of set_camera!

[mattuntergassmair]

oh! I did not see that render! disappeared. This makes much more sense now. An example would definitely help.

Now the usage would be

update_camera!(cam, scene_or_whatever_is_needed)
render(renderables, camera-cam)

Yes exactly essentially now with CameraState we have Zac's nice simple interface back again

[mattuntergassmair]

set_camera! is just internal then? The rendermodel still needs to know about the CameraState right? How is this done?

The camera is passed in together with the rendermodel when calling render_to_canvas. This is the only time the camera needs to meet the rendermodel.

[mattuntergassmair]

I've been having difficulties making julia run as a Jupyter notebook so couldn't really work with the tutorials, but here is how I use the new interface in my own DenseTrafficMerging code:

# camera = TargetFollowCamera(:ego, y=0., zoom=10.)
camera = SceneFollowCamera()
for scene in scenes
    update_camera!(camera, scenes[i+1])
    renderables = [roadway]
    for veh in scene
        push!(renderables, FancyCar(entity=veh, color=colorant"red")
    end
    render(renderables, camera=camera)
end

[MaximeBouton]

What is the status of this? I would say, add a minimum of documentation then I can review it and we can merge this PR?

[mattuntergassmair]

What is the status of this? I would say, add a minimum of documentation then I can review it and we can merge this PR?

I'd say it's ready. Would be nice if we could get the Travis CI build to work again, but I don't know what is the problem with that

Not much has changed from the user point of view, the camera state is mostly used internally. What kind of further documentation do you suggest adding?

I had trouble running the Jupyter notebooks but having those reflect the newest changes would be useful

[MaximeBouton]

in terms of documentation, the minimum would be a simple example as the one in your previous comment.

[mattuntergassmair]

in terms of documentation, the minimum would be a simple example as the one in your previous comment.

Ok, I added some documentation for example useage. The docs at the moment are pretty sparse and documenting it properly would easily take several hours. We should probably aim to make the docs more comprehensive and move them from the README to to an actual documentation page, but that endeavour could easily have its own PR.

[MaximeBouton]

Agreed, the docs deployment is already set up but it would involve some writing for sure