Update windowing chapter #17
Conversation
|
|
||
| let window = surface | ||
| .object() | ||
| .unwrap() | ||
| .clone() | ||
| .downcast::<Window>() | ||
| .unwrap(); |
There was a problem hiding this comment.
This is duplicated since it also appears below. I like that put a separate version of this together with the explanation, so I think it's best if you stick only with the below one and remove this one.
| let window = surface | |
| .object() | |
| .unwrap() | |
| .clone() | |
| .downcast::<Window>() | |
| .unwrap(); |
| for rendering. As for the window itself, it can be retrieved by calling `surface.window()`, which | ||
| for rendering. As for the window itself, it can be retrieved this way: | ||
|
|
||
| ```rust |
There was a problem hiding this comment.
Window is not in scope.
| ```rust | |
| ```rust | |
| use winit::window::Window; | |
| .object() | ||
| .unwrap() | ||
| .clone() | ||
| .downcast::<Window>() | ||
| .unwrap(); |
There was a problem hiding this comment.
fmt
| .object() | |
| .unwrap() | |
| .clone() | |
| .downcast::<Window>() | |
| .unwrap(); | |
| .object() | |
| .unwrap() | |
| .clone() | |
| .downcast::<Window>() | |
| .unwrap(); |
|
|
||
| which | ||
| you can use to manipulate and change its default properties. |
There was a problem hiding this comment.
This continues a sentence from above the code block, which is generally a no-no as it can be a bit hard to read.
More importantly however, ideally users wouldn't use this procedure every time they need to access the window. This is something users seem to be doing and so I would like to make this distinction clearer. I'm not sure what the best phrasing is myself (feel free to comment).
All in all, maybe it's best to remove this entirely.
| which | |
| you can use to manipulate and change its default properties. |
There was a problem hiding this comment.
Update: the issue has just been resolved upstream. Therefore there is no further clarification needed, as this way of creating the surface is going to be deprecated in the next release.
| .then_execute(queue.clone(), | ||
| command_buffers[image_i as usize].clone()) |
There was a problem hiding this comment.
fmt
| .then_execute(queue.clone(), | |
| command_buffers[image_i as usize].clone()) | |
| .then_execute(queue.clone(), command_buffers[image_i as usize].clone()) |
| @@ -206,7 +205,7 @@ also have multiple framebuffers, we will have multiple command buffers as well, | |||
| framebuffer. Let's put everything nicely into a function: | |||
|
|
|||
| ```rust | |||
| use vulkano::buffer::TypedBufferAccess; | |||
| use vulkano::buffer::subbuffer::Subbuffer; | |||
There was a problem hiding this comment.
I would personally prefer importing the re-exported one from the buffer module as that's more succinct. Also, the StandardCommandBufferAllcoator used below is not in scope.
| use vulkano::buffer::subbuffer::Subbuffer; | |
| use vulkano::buffer::Subbuffer; | |
| use vulkano::command_buffer::allocator::StandardCommandBufferAllocator; |
|
There seems to be one use of the old |
|
Hi! If you need anything please let me know. No rush at all though. :) |
|
Thanks! Sorry, I've just been busy. I should be able to go through this stuff soon! |
Resolves #10. I made the mentioned changes as well as more updates to the example code in the chapter. Let me know if anything needs adjustment.