iPlug2 / iPlug2

C++ Audio Plug-in Framework for desktop, mobile and web
https://iplug2.github.io
Other
1.89k stars 283 forks source link

Nested IControls #590

Open Bindernews opened 4 years ago

Bindernews commented 4 years ago

I've seen several people mention the desire for nested IControls previously (including issue #41 ), so I figured I'd make a larger issue about it for the purpose of discussion. I think these are the major points to consider w.r.t. nesting IControls:

Flat Model

Pros

Cons

Tree Model

Pros

Cons

I'm willing to write the code but I think this is a pretty major feature, and it needs to be addressed.

NOTE: If anyone posts more pros/cons, I'll update this post to include them.

AlexHarker commented 4 years ago

I'm pretty sure that the tree model is preferable, although I'd prefer it not to be breaking and I'm not convinced that it needs to be. For me the issue is that most of what is currently in IGraphics would go into something like a container or viewport and then Graphics would either have a default one (composition) or inherit the features of one - not sure which is better or easier. Then a lot of methods in IGraphics (e.g. mousing/drawing) would simply call into the main containter/viewport, possibly with some coordinate management beforehand. We could then use transforms to have local coordinates and zoom/scale within a viewport, although ensuring these worked correctly would be a challenge due to the way transforms are managed currently.

The recent I haven't done this is simply

If you are willing to take a shot at it, and also willing to discuss it in more detail I'd suggest a longer discussion off list, and possibly with at least an audio conversation to make it easier. There's a lot of detail I could go into there that would be hard to do or extremely tedious in written form.

Obviously, input from @olilarkin would be good to make sure he'd be onboard with a plan.

Bindernews commented 4 years ago

I agree that we can make the changes without breaking the API but I think we may end up deprecating some methods, which would (at a later time) mean breaking changes. I'm willing to take a shot at it but not until mid-August. I just wanted to start the conversation now b/c I know these things take time.

I definitely agree we should start a more in-depth discussion, but I also want to ensure that other users have the opportunity to voice their opinions. I suspect most people will want the tree structure, but maybe I'm wrong. We can figure out a time for an audio call later, once I'm done with my current commitment.

AlexHarker commented 4 years ago

Sounds good.

olilarkin commented 4 years ago

It would be great to solve this, and wouldn't be too upset having to modify my product codebases if necessary to deal with changing to a tree model. I am slightly bothered by the CPU usage of IGraphics that I see in Xcode, with fairly minimal animation at 60 FPS, which is often around 7%. I am not sure if switching to a more sophisticated tree model and changing the way dirty regions are specified can improve this or not, or if this is even something worth worrying about. Whenever I profile it I normally see e.g. a nanovg draw function is the hotspot.

There is a branch here where I have been working on a new multi-touch capable IVKeyboard control that nests individual controls for each key. Seems mad to redraw the entire keyboard if only a single key is pressed.

https://github.com/iPlug2/iPlug2/tree/controls/multitouch_controls

This is kind of a hack though, a bit like the IVNumberBox control, which easily breaks down when you try and move/resize the control programmatically.

@Bindernews I would be very interested to see a proposal for a tree solution, if you have time to work on something.

Bindernews commented 4 years ago

@olilarkin What do you want in a proposal? Do you mean a PR, or more of a proposal of new/removed/changed methods and functionality that we can discuss in a call?

Slightly off topic: why are things named SetDisabled and Hide? It seems weird have a kind of double-negative "No, don't hide this" in order to show it. I'd say change those to SetEnabled and Show respectively each of which can call SetDisabled and Hide for backwards-compatibility.

olilarkin commented 4 years ago

Just if you have any ideas of a better way that it should work, even if quite breaking, you could make a branch for us to try, for discussion, not a PR as such.

Up for renaming methods more sensibly, make an issue if you like

AlexHarker commented 4 years ago

FWIW - I think they are called SetDisabled and Hide because controls default to enabled and shown. So it is an active choice to do the opposite. I see how SetDisabled() is more problematic if you want to enable (I see a big issue with unhiding with a call to Hide(false).

Hide is historical - SetDisabled I think used to be SetGrayed().

For my money doubling methods is not a great idea (worse than having objectionable names) and making the change will reverse the logic in anyone's plug-ins, leading to a breaking change, with a likelihood of introducing bugs.

asedev commented 4 years ago

Since C++14 one is able to use this feature:

[[deprecated("Replaced by SetEnabled, due to ...")]]
void SetDisabled(bool v) {
  SetEnabled(!v);
}

void SetEnabled(bool v) {
  enabled = v;
  ...
}
olilarkin commented 3 years ago

Some IContainer work here:

https://github.com/iPlug2/iPlug2/commit/1de55f385bb6340a038dc29e63d1dd3ef848cd65

AlexHarker commented 3 years ago

Can we close #41 now as this seems more up to date and references #41 should anyone wish to check it?

olilarkin commented 3 years ago

There is also an experiment here I did a while ago: https://github.com/iPlug2/iPlug2/commits/controls/child_controls