Patch types

[CorvusPrudens]

Note: I've made this a draft PR because there's a little bit more documentation work that needs to be done if we adopt this approach. I also need to carefully review each node to make sure I haven't changed their behavior.

This PR introduces explicit patch types, which live as associated types on the Patch trait. To update a struct from a Firewheel event, Patch implementations must now construct their associated type and then apply it.

pub trait Patch {
    type Patch;

    fn patch(data: &ParamData, path: &[u32]) -> Result<Self::Patch, PatchError>;

    fn apply(&mut self, patch: Self::Patch);
}

This might seem a little odd at first, but this approach has some major upsides.

First, audio processors can return to reacting to individual field updates just like earlier version of Firewheel. Only this time, users can work with a much easier, type-safe enum:

// examples/custom_nodes/src/nodes/filter.rs

#[derive(Diff, Patch, Debug, Clone, Copy, PartialEq)]
pub struct FilterNode {
    pub cutoff_hz: f32,
    pub volume: Volume,
    pub enabled: bool,
}

// The processor no longer needs a `FilterNode` field
struct Processor {
    filter_l: OnePoleLPBiquad,
    filter_r: OnePoleLPBiquad,
    cutoff_hz: SmoothedParam,
    gain: SmoothedParamBuffer,
    enable_declicker: Declicker,
    sample_rate_recip: f32,
}

impl AudioNodeProcessor for Processor {
    fn process(
        &mut self,
        buffers: ProcBuffers,
        proc_info: &ProcInfo,
        mut events: NodeEventList,
    ) -> ProcessStatus {
        events.for_each(|e| {
            // `patch_event` is just a convenience method wrapping `patch`
            match FilterNode::patch_event(e) {
                // Here, we can match on `FilterNodePatch`, the `Patch` type generated by the `Patch` macro.
                // Notice how we do only as much processing as we need for each event, instead of re-calculating
                // everything if any event was received.
                Some(FilterNodePatch::CutoffHz(cutoff)) => {
                    self.cutoff_hz.set_value(cutoff.clamp(20.0, 20_000.0));
                }
                Some(FilterNodePatch::Volume(volume)) => {
                    self.gain.set_value(volume.amp_clamped(DEFAULT_AMP_EPSILON));
                }
                Some(FilterNodePatch::Enabled(enabled)) => {
                    self.enable_declicker
                        .fade_to_enabled(enabled, proc_info.declick_values);
                }
                _ => {}
            }
        });
    }
}

Second, you can still keep a Node type around and apply patches to it, but you're free to intercept the patches and enforce invariants without disrupting diffing and patching in the main thread.

// crates/firewheel-nodes/src/spatial_basic.rs

// ...
events.for_each(|e| {
    let Some(mut patch) = SpatialBasicNode::patch_event(e) else {
        return;
    };

    // Here, we make sure our audio processor doesn't observe certain conditions
    // by matching on the patch mutably.
    match &mut patch {
        SpatialBasicNodePatch::Offset(offset) => {
            if !offset.is_finite() {
                *offset = Vec3::default();
            }
        }
        SpatialBasicNodePatch::MuffleCutoffHz(cutoff) => {
            *cutoff = cutoff.clamp(DAMPING_CUTOFF_HZ_MIN, DAMPING_CUTOFF_HZ_MAX);
        }
        SpatialBasicNodePatch::PanningThreshold(threshold) => {
            *threshold = threshold.clamp(0.0, 1.0);
        }
        _ => {}
    }

    // Then, we simply apply it.
    self.params.apply(patch);
    updated = true;
});

Additionally, with types like Notify, you can react to events like events. In other words, you can reliably trigger behavior like moving a sample playhead without additional effort or synchronization.

Finally, if your type doesn't need any of this, you can just ignore all the new features and use it like before.

// examples/custom_nodes/src/nodes/rms.rs

// ...
self.params.apply_list(events);

Overall, this seems like a huge win-win.

At first glance, you might think this would perform a bit worse than applying mutations directly from the events. However, the benchmarking revealed no changes between the previous patch trait and this one -- even for deeply nested types. It appears that LLVM has an easy time optimizing out the construction of patch types.

Footnote: Diffing and patching A with B should make A == B

If we want to reliably send the minimum amount of data to synchronize normal code and audio code, we need a way to make two pieces of data equal after diffing. Previously, bevy_seedling cloned after diffing. This is simple, but may not be efficient depending on what's being cloned.

Since publishing, however, bevy_seedling now relies on diffing and patching to rectify two pieces of data. We built patching to be highly efficient and fine-grained, so it is generally much more efficient to apply patches to a struct than to clone it. However, this means that patching must make two structs equal. Enforcing invariants in Patch breaks this relationship.

Luckily, we no longer need this half-measure. Since it's now much easier to work with a patch before it's applied, invariants can be upheld in audio processors directly. Consequently, the only time users will need to implement Patch manually is when they're defining leaf types. This should be very rare, since we've already defined most of these (e.g. f32, i32, etc.).

[CorvusPrudens]

After creating this PR, I realized that we can make a slightly more ergonomic method for iterating over patches. Something like:

events.for_each_patch::<FilterNode>(|p| match p {
    FilterNodePatch::CutoffHz(cutoff) => {
        self.cutoff_hz.set_value(cutoff.clamp(20.0, 20_000.0));
    }
    FilterNodePatch::Volume(volume) => {
        self.gain.set_value(volume.amp_clamped(DEFAULT_AMP_EPSILON));
    }
    FilterNodePatch::Enabled(enabled) => {
        self.enable_declicker
            .fade_to_enabled(enabled, proc_info.declick_values);
    }
});

Let me know if this seems worth adding!

We could replace the apply_list-style methods on Patch with this approach as well, which may help users understand what's going on a little better while cutting down on our API surface.

events.for_each_patch::<FilterNode>(|p| self.params.apply(p));

[CorvusPrudens]

After rebasing on main, I've integrated the new derive macros with the sampler node. This should have the same performance characteristics as before (the same number of allocations are created), but now we can patch SamplerNode. Note that I also wrapped PlaybackState in the new Notify type. This is because the user may want to restart playback of a short sample multiple times. Without notify, repeatedly writing Play { .. } would not be caught.

Detecting sequence completion

When integrating playback control with bevy_seedling, however, I had trouble reliably detecting when non-looping sample playback actually completed. This is critical for resource cleanup in the ECS.

This is largely due to the fact that a stopped flag is not enough information to determine whether a sequence has completed. There is some delay between starting playback in the ECS and the stopped flag being cleared, so we might accidentally remove a sample before it's even started playing. To mitigate this, we could poll the flag in the ECS, waiting for it to be false and then storing the fact that playback has started, but this mechanism cannot handle samples that last for less time than the polling frequency. Also, we might just stop playback to pause a sample, which only adds more difficult-to-handle scenarios.

My solution was to add a specific finished flag to the sampler's shared state. This can only be set outside the sampler, and it is only cleared when the sequence completes. This has been much more reliable for me.

This solution is not perfect, however. If the sample pool is at max capacity and we choose to play a new sample with a sampler that's currently running, there could be a condition where we clear the flag, set the new sequence in the ECS, and before the event arrives at the sampler node, it finishes its old sequence and sets the flag. The ECS will then interpret this as the new sequence being completed, even thought it hasn't played.

This error case could be mitigated with another mechanism. Sequences could have an ID, acquired in the same way that Notify acquires unique counter values. When a sequence finishes, the sampler node could write this ID to a shared atomic. However, this approach seems annoying and fiddly for non-Bevy users, so it might not be worth exploring.

[CorvusPrudens]

This PR is a little large, so I could separate out the sampler node changes into a new PR. I've kept it all in one PR because it's easier for me to integrate and verify the new behavior in bevy_seedling this way.