Skip to content

Comments

Midi#77

Merged
tychedelia merged 8 commits intoprocessing:mainfrom
catilac:midi
Feb 24, 2026
Merged

Midi#77
tychedelia merged 8 commits intoprocessing:mainfrom
catilac:midi

Conversation

@catilac
Copy link
Contributor

@catilac catilac commented Feb 23, 2026

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.

@catilac catilac requested a review from tychedelia February 23, 2026 21:02
Render,
send_view_targets
.in_set(RenderSystems::ManageViews)
.in_set(RenderSystems::Specialize)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lacks wisdom. I think this is the correct schedule?

Comment on lines +296 to +298
// 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just a few small comments.

use bevy::prelude::*;

#[derive(Debug, Default, Component)]
pub struct CommandBuffer<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@tychedelia tychedelia merged commit 5e2c67a into processing:main Feb 24, 2026
1 of 4 checks passed
@catilac catilac deleted the midi branch February 24, 2026 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants