sisl / AutoViz.jl

Provides visualization tools for AutomotiveDrivingModels. Built on Cairo
Other
33 stars 9 forks source link

Initial PR for v0.8 #33

Closed mattuntergassmair closed 4 years ago

mattuntergassmair commented 4 years ago

Some major restructuring of the code base, interface changes and deprecations. For a more detailed list of changes please consult the Changelog section in README.md.

mattuntergassmair commented 4 years ago

Closes #9

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-13.009%) to 37.78% when pulling ba1fb0a248ce7ad27a33e32689f22d430effb4c6 on mattuntergassmair:v0.8 into d94d04bfb6e4551de5b2b5e66dbc442eacee2fd1 on sisl:v0.8.

MaximeBouton commented 4 years ago

Questions on the camera feature:

mattuntergassmair commented 4 years ago

Questions on the camera feature:

* Should we get rid of `StaticCamera` since it does nothing?

* Why would `update_camera!` always need the scene? Would it make more sense to have `update_camera!(rm, camera)` and the camera object would contain all the information? e.g. `TargetFollowCamera` would have the entity as part of its field and `SceneFollowCamera` would have the scene.

Good points! As far as StaticCamera, I agree that we could get rid of it in terms of functionality. The reason I kept it around is because it is a "dummy" camera that satisfies the same interface as other cameras. If we didn't have StaticCamera, but we wanted a camera that "does nothing", we would have to set camera_motion = nothing and then do stuff like if camera_motion === nothing ... else ... end. So my preference was to just have a static camera that follows the same interface but does nothing. But there may be better ways of doing this and we could get rid of StaticCamera alltogether.

As for the update_camera interface, it would be great to only take a rendermodel and camera and have the rest as fields of the camera. Say if target_vehicle or scene were members of TargetFollowCamera and SceneFollowCamera respectively, how would we keep them up to date? Would we re-initialize a new camera in every time step of the visualization, via cam = SceneFollowCamera(scenes[i])? If we had a SceneFollowCamera(scene) that we keep around over several time steps, we would need to make sure scene stays up to date.

mattuntergassmair commented 4 years ago

The two main points remaining are

  1. make the camera interface nicer
  2. provide a better default function for rendering These will be addressed in a future PR, merging this one for now