-
Notifications
You must be signed in to change notification settings - Fork 9
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
Initial PR for v0.8 #33
Changes from all commits
ea0bb8f
1485835
f1bd17d
8b7a64b
173a08b
321f13a
43648bd
d41e592
7726c84
1c0a81b
ba1fb0a
d0aac34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
*.jl.cov | ||
*.jl.mem | ||
**/.ipynb_checkpoints | ||
docs/build/ | ||
docs/build/ | ||
|
||
test/*.svg |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -115,3 +115,40 @@ using AutoViz | |
set_color_theme(LIGHTTHEME) | ||
``` | ||
You can also define your own color theme using a dictionary. Look at the example in `src/colorscheme.jl` to have the correct key names. | ||
|
||
|
||
## Change Log | ||
|
||
### v0.8.x | ||
|
||
#### Rendering | ||
- Clean-up of the rendering interface: there is now only one single rendering function with the signature | ||
``` | ||
render!(rendermodel::RenderModel, renderables::AbstractVector; canvas_width::Int, canvas_height::Int, surface::CairoSurface)) | ||
``` | ||
All keyword arguments are optional. Objects of type `Renderable` now no longer have to implement the `render!` function (which is a misleading name). Instead one must implement the `add_renderable!` function which adds the rendering instructions to the `RenderModel`. | ||
- Implicit conversions of non-renderable objects (such as `obj::Frame{Entity{S,D,I}}`) via implementations of `Base.convert(Renderable, obj)` are now discouraged. Instead, one can overwrite the `add_renderable!` method for such types. This is done for some very common types. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, I agree |
||
- The new `render!` function now only takes objects which are renderable, i.e. which implement the `add_renderable(rm::RenderModel, obj)` function. There is no longer a distinction between drawing roadways, scenes or overlays. They all need to satisfy the same interface, and they are drawn in the order in which they are passed to the `render!` function. This change decreases the number of available render functions from almost ten to one and should make the control flow more clear. | ||
- Additional arguments to `render!` such as `camera` and `car_colors` are no longer supported. Camera effects should be applied before calling `render!` (see section below) and rendering attributes such as colors should be passed in as part of a renderable object. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yes! |
||
|
||
#### Overlays | ||
- Changed the interface for rendering overlays to only take an instance of `RenderModel` and the overlay itself. All additional data must be stored as part of the overlay if it is needed during rendering. | ||
- Added a `RenderableOverlay` wrapper which makes the legacy overlays work with the new rendering interface (in which overlays do not get any input arguments for rendering) | ||
|
||
#### Cameras | ||
- Changed the camera interface. The full state of the camera, such as `camera_pos`, `camera_zoom`, `camera_rotation` is stored in `RenderModel` (this has already been the case in previous AutoViz versions). A `Camera` acts upon a `RenderModel` by changing these internal variables. The function `camera_set!` now becomes `update_camera!`. | ||
- Many setter functions for the camera have been replaced by the `set_camera!()` function which takes keyword arguments for `x`, `y` and `zoom`. | ||
- The implementations of `TargetFollowCamera` (former `CarFollowCamera`) and `SceneFollowCamera` have been reviewed and simplified. Additionally, a `ZoomingCamera` type which gradually changes the zoom level has been introduced and for easy extensibility there is also a `ComposedCamera` type which takes a list of cameras and applies their effects sequentially to the `RenderModel`. | ||
- The new `render!` function no longer takes a camera as an input argument, but assumes that the camera settings have already been applied to the `RenderModel` via `update_camera!` prior to calling `render!`. User code should be adapted accordingly. | ||
|
||
#### Visualization of Entities | ||
- Controlling the appearance of vehicles by setting `set_render_mode(:basic|:fancy)` is no longer encouraged. Instead, we provide new renderable types such as `EntityRectangle`, `FancyCar`, `FancyPedestrian`, `VelocityArrow` in addition to the already implemented `ArrowCar` type which can all be used to conveniently display entities. | ||
- A convenience function for rendering scenes directly (i.e. without explicit conversion to a `Renderable` type) is still supported. | ||
- TODO: make FancyCar work on my platform | ||
|
||
#### 1D Vehicles | ||
- Support for 1D vehicles has mostly been discontinued and some of the related functions were removed. However, the new functions should work seamlessly in many cases as long as the 1D vehicles implement basic functions such as `posg`, `width`, `length` from `AutomotiveDrivingModels.jl` | ||
|
||
## TODO: adapt tutorials | ||
## TODO: adapt unit tests | ||
## TODO: adapt docs |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean the user has to construct a
RenderModel
object explicitly?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think the typical usage now is as follows:
I think it would still be convenient to have one
render
function that allows to do things in one function call and have the explicit version if one wants more fine tuning.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,
RenderModel
needs to be constructed explicitly. This is mostly becauseRenderModel
holds the main camera parameters (camera_center
,camera_zoom
,camera_rotation
). This is not by my choice but rather howRenderModel
was designed to work. In fact if designing from scratch I would probably not have theRenderModel
hold those camera parameters.So in order to set the initial zoom level (for example), it needs to be passed to the
RenderModel
. In the previous version, the zoom level was set by first setting the zoom level of a camera and then the camera would go set the zoom level of the rendermodel, but effectively those variables were duplicated inRenderModel
andCamera
which I was not a big fan of. Would you agree?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a conceptual level, the camera state really lives inside the
RenderModel
(again, based on legacy code) and the structs that are calledCamera
actually describe camera motions over time (like "stay static", "follow a vehicle", "follow the scene"). This is quite subtle but I think the struct names may be a little misleading.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth providing one or two convenience functions that take care of constructing a
RenderModel
but for full control I think we need all three steps (RenderModel
,update_camera!
,render!
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MaximeBouton added an "easy" render function that just takes a scene and optional overlays. There are still a few open questions there regarding the defaults
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try it out today.
What would be your suggestion on how to handle the camera if we did not have the
RenderModel
struct?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the default render method this is what I had in mind:
no need to go back to something specific that takes
scene
androadway
as input.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One Idea that crossed my mind would be to have a
CameraState
object that has the fieldszoom
,position
,rotation
. EachCamera
then has a reference to aCameraState
which it then manipulates. And when we pass in theCamera
to therender
function, with it comes theCameraState
object. In addition to theCameraState
,Camera
itself would hold other variables that parameterize how it moves over the scene (target_id
,n_vehicles
in scene, etc).RenderModel
would then only keep track of the rendering instructions essentially (maybe the canvas dimension and canvas surface too, didn't spend much thought on this).RenderModel
andCameraState
would only be combined in the finalrender_to_canvas
function.In this way, we would not need to pass in the
RenderModel
torender
but only theCamera
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish this were possible, it was also what I initially started off with. But I currently don't see an easy way of doing this:
SceneFollowCamera
andTargetFollowCamera
both need access to the scene in order to determine their new position (this is why we had issue SceneFollowCamera in new interface #9 whereSceneFollowCamera
would not work in the new rendering interface). The best we could do would be to default toStaticCamera
but not sure if the convenience is worth what we're giving up.The only way i see around that would be to give every renderable object a depth parameter which we can query using
depth(obj)
and then determine rendering order based on that. Still not really the way to go I think :/So given these two constraints, this "convenience" render function was the best I could think of...