Conversation
| Render, | ||
| send_view_targets | ||
| .in_set(RenderSystems::ManageViews) | ||
| .in_set(RenderSystems::Specialize) |
There was a problem hiding this comment.
This lacks wisdom. I think this is the correct schedule?
| // TODO: Setting this as a default, but we need to think about API around | ||
| // a user defined value | ||
| let near_clip_plane = vec4(0.0, 0.0, -1.0, -near); |
There was a problem hiding this comment.
I'm putting value here for now. But after this gets merged, we'll need an issue to create intentional API around near_clip_plane. We want to support portals :P
tychedelia
left a comment
There was a problem hiding this comment.
LGTM! Just a few small comments.
crates/processing_utils/src/lib.rs
Outdated
| use bevy::prelude::*; | ||
|
|
||
| #[derive(Debug, Default, Component)] | ||
| pub struct CommandBuffer<T> { |
There was a problem hiding this comment.
I'm not sure this makes sense to pull out. The reason we buffer commands for rendering is because we want to avoid submitting to the GPU any more than we have to. But for other hardware, it's almost always going to be the right call to just "do the thing" right when the user asks. In other words, command buffering is a kind of behind the scene optimization for users where we can take advantage of the fact that nothing needs to be executed until something becomes visible to the user. Because we don't have the same abstraction of "the frame" for stuff like music/midi, it makes sense just to issue the command immediately.
There was a problem hiding this comment.
ahhh i see. okay this abstraction is unnecessary then. i'll take it out! though i'd like to land for the bevy updates.
|
|
||
| pub fn connect(port: usize) { | ||
| // we need to work with the ECS | ||
| // do we pass a MidiCommand to Bevy? |
There was a problem hiding this comment.
I think we need to make a few changes to bevy_midi to support things like hot-plugging. Right now it assumes that devices are available on startup.
There was a problem hiding this comment.
Ahhh okay good to know! in this PR the midi stuff is so early because of the bevy updates. i was going to keep working on this, but i'm wondering if we could land the bevy changes just so main has them
This is a WIP for Midi, but the requirements has triggered the update of Bevy to the latest dev version 0.19. It's exciting, but means breaking changes.
I've attempted making most of those fixes, and when I put up the PR I'll comment on the lines that lacked wisdom from my part in case that needs to be updated.