Frando / rust-faust

Easily use FAUST DSPs in Rust
Apache License 2.0
56 stars 12 forks source link

Proposition: Richer node type #12

Closed plule closed 2 years ago

plule commented 2 years ago

This is a much more intrusive change than #10, and replaces it, but with the same intention: Expose all the DSP parameters data to the user.

This PR replaces the couple NodeKind/Option<Props> with a richer WidgetKind.

The widget kind content has two levels:

Program iterating over the values can either:

plule commented 2 years ago

And a quick and dirty poc using this PR for building a generic UI (as long as there is not too much, I've not done any layout): https://github.com/plule/faust-egui/blob/main/src/ui.rs

image

Jikstra commented 2 years ago

Thank you! I like this and definitely that part of the code needed some work/redesign. One thing I'm not so sure about is having the two levels. Can you elaborate the benefits of those two levels? The only thing i see is that it could help in guis that only have more generic widgets?

plule commented 2 years ago

I was thinking about situation where you are not building a UI from the controls, but controlling them from another kind of input (midi, hardware, ...). In that situation, the difference between "horizontal slider", "vertical slider" and "num entry" is not relevant.

This helps writing code like this:

let ranged_input = if let WidgetType::RangedInput(ranged_input) = node.widget_type() {
    ranged_input
} else {
    panic!()
};

instead of a more complicated match:

let ranged_input = match node.widget_type() {
    WidgetType::HorizontalSlider(ranged_input) => ranged_input,
    WidgetType::VerticalSlider(ranged_input) => ranged_input,
    WidgetType::NumEntry(ranged_input) => ranged_input,
    _ => panic!()
}

But it probably just complicates things for not much benefits, I'll add a new commit with a flat enum to see how it looks!

Jikstra commented 2 years ago

But it probably just complicates things for not much benefits, I'll add a new commit with a flat enum to see how it looks!

I think if this really proofs to be something that we need, we can still have a WidgetType.kind() function returning this enum type? But for me the flat variant looks better. Will check it out and have a closer look :)

plule commented 2 years ago

Yep that flat enum is much more intuitive. For the helper function, I don't think a .kind() function is sufficient: we would have to add a new enum, and it would not give the caller back the information they need (init/range/...).

Just throwing in random ideas, maybe a TryFrom implementation for RangedInput and RangedOutput?

In faust-type:

impl<'a> TryFrom<&'a WidgetType> for &'a RangedInput {
    type Error = &'static str;

    fn try_from(value: &'a WidgetType) -> Result<&'a RangedInput, Self::Error> {
        match value {
            WidgetType::VerticalSlider(input) => Ok(input),
            WidgetType::HorizontalSlider(input) => Ok(input),
            WidgetType::NumEntry(input) => Ok(input),
            _ => Err("Not a RangedInput"),
        }
    }
}

and the corresponding client code:

let ranged_input: &RangedInput = node.widget_type().try_into().unwrap();

Or just try_get_ranged_input(&self) -> Option<&RangedInput>/try_get_ranged_output(&self) -> Option<&RangedOutput> /is_boolean_input(&self) -> bool to make it more discoverable, and covers the "special case" of the boolean input that does not have any data with it.

Jikstra commented 2 years ago

Let's merge and figure out the kind stuff on the way :) Thank youuuuu for the PR

Frando commented 2 years ago

Great, I like this change a lot too :slightly_smiling_face: Thanks for working on this!