First version of a new dynamic LADSPA control dialog

[michaelgregorius]

The new dialog shows the LADSPA controls in a matrix layout. Each row corresponds to a LADSPA parameter. Each parameter can have several channels which can be linked. Each channel has its own row of controls.

Some base classes have been extended to enable the resizing of the new control dialog. A new virtual method isResizable was added to the class EffectControlDialog. The default implementation returns false which means that for all other types of dialogs the code in EffectView.cpp will still set the dialogs to a fixed size. LadspaMatrixControlDialog returns true so the code is skipped. A better solution would be to remove the fixed size code from EffectView.cpp altogether and to make it an implementation detail of each dialog whether it is resizable or not.

The class LadspaMatrixControlView was added by copying and modifying LadspaControlView to get rid of the link buttons which are associated with some controls and some labels.

Here is a screenshot showing the dialogs in action:

[diizy]

looks good, but we're under feature freeze so this'll have to wait until we separate 1.2 from master

[Wallacoloo]

The Calf EQ currently looks like this for me:

I have to say, I like the current look better than the look in your screenshot. I'm not familiar with the code used to create it though - are the knob positions currently manually programmed and your change makes these dynamic and automatic? If so, excellent and :+1:.

But I don't think having one parameter per row looks right. For example, it makes quite a bit more sense to have "HP Active", "HP Freq" and "HP Mode" on a single line, as they are currently, rather than split across rows like this. Is this part of the end goal?

[michaelgregorius]

Hi @Wallacoloo,

the knob positions and the layout are computed dynamically in both implementations. But I think the layout in my new implementation is a bit more tidy. For a comparison I have attached a screenshot with the other two dialogs:

Please notice that all of my screenshots have been made on a display with a DPI of 117.5 so the font may look rather large on other screens. Here is what I find inadequate about the current implementation:

• For plugins with more than one channel the parameter names are repeated which adds to clutter.
• Knobs do not always line up which also adds to a cluttered look.
• Having the link button for each parameter next to the control of the first channel made the layout/groups look unbalanced.
• The unbalanced layout makes it harder to find a corresponding control in another channel because the first and the second channel do not look identical.

I agree that having "HP Active", "HP Freq" and "HP Mode" on a single line is a better grouping in the case of the Calf EQ. This is something that could be improved in the new implementation. I have to check the old code again to see whether LADSPA gives an indication of what should be considered a group and what not. I can also imagine later implementations to have some layout configuration file which describes for a given plugin what type of control should be used for a parameter (knob, slider, etc.) and how they are grouped or layouted. But that's quite some big undertaking which should not be part of this pull request (one step at a time).

[Wallacoloo]

@michaelgregorius I agree with all the inadequacies you pointed out in the current implementation. Given the direction you intend to continue with this, I'm in support of merging this (after 1.2 is forked). The code looks good - no glaring style issues. Some sections could be commented better, but that's me being nitpicky.

[badosu]

Hi @michaelgregorius, really nice to see someone taking a look at the LADSPA view.

These issues might interest you in future iterations:

• Supporting output meters (https://github.com/LMMS/lmms/issues/75)
• Support additional control data (https://github.com/LMMS/lmms/issues/1717)

[Wallacoloo]

Seeing as how we have not been enforcing a feature freeze on any other PRs and we have no clear timeline for procuring a 1.2 release candidate, should we merge this one?

[tresf]

No objections here.

[budislav]

I don't want to be boring with this but in case you didn't notice my work on LADSPA plugins in single-window UI

Old control dialogs are better because of horizontal scrolling. New matrix layout is not good for single-window because dialogs have fixed height.

[Wallacoloo]

@budislav I don't follow 100%. Take the old Calf EQ layout shown in @michaelgregorius 's comment earlier: https://github.com/LMMS/lmms/pull/2068#issuecomment-105162567 . As is, it seems that both the new implementation as well as the current one are ill-fitted to being placed in a horizontal scrolling environment like that in your proposal.

For what it's worth, I don't imagine that very much of the existing LADSPA layout code will be compatible with your UI proposal anyway. Unless I'm mistaken, that portion of it will pretty much be a rewrite when based against either the current code or the code in this pull request. So in my opinion it's more important to question whether this pull request will improve the existing UI and/or make the code easier to deal with in the future.

I don't mean to discredit your UI proposal or to offend - my perspective is one in which I value small, incremental improvements in order to make growth more manageable at the cost of these improvements occasionally being redundant.

[michaelgregorius]

@Wallacoloo I agree 100% on the value of small, incremental improvements and doing things in a rather agile way. I also agree that this code is something that will have to be rewritten anyway due to the dynamic nature of the dialogs.

@budislav Are you aware that the LADSPA dialogs have dynamic layouting, i.e. they are build at runtime and that it is not possible to design them upfront as in your examples?

[tresf]

@budislav Are you aware that the LADSPA dialogs have dynamic layouting, i.e. they are build at runtime and that it is not possible to design them upfront as in your examples?

FYI - His layout is dynamic too. His W/D, DECAY and GATE are from the effects chain.

His channel stack is already being performed.

... and his linking icon should be possible as well.

So I think his request is rather valid from a layout perspective. It's good foresight to take the future designs into consideration, even if we don't code them now.

[budislav]

@tresf thank you. Exactly. @Wallacoloo for LADSPA layout every UI pattern like checkbox or slider would be placed one after the other but because of fixed height everything will just go to the right. Width is not fixed in this case. @michaelgregorius I don't see anything impossible here. It only require gui logic.

[Wallacoloo]

for LADSPA layout every UI pattern like checkbox or slider would be placed one after the other but because of fixed height everything will just go to the right. Width is not fixed in this case.

@budislav I understand that part. Perhaps I misunderstood your earlier post. I was under the impression that you were against merging this pull request, specifically for the reason that the height of the layout is not fixable in @michaelgregorius's implementation. This confused me because one cannot to my knowledge fix the height in the current implementation either, and if that's correct, then how could one use that as a reason to advocate against this PR?

So I have two clarifying questions to ask you:

• Was your post intended as an objection to merging this pull request?
• Can one fix the height of the existing layout implementation? (I would check myself, but I don't have the facilities to run LMMS at this moment)

And if your answers are "yes", followed by "no", then I'd also ask that you elaborate on why you think this pull request is a downgrade from the current implementation.

[budislav]

@Wallacoloo

Was your post intended as an objection to merging this pull request?

No, I misunderstood, it's my fault. I thought that these changes would be unnecessary because in single-window concept LADSPA is designed in mind of present design (no matrix).

[michaelgregorius]

@budislav I guess it will take quite some time to implement the new GUI and as far as I know there are no concrete plans when this will be done/started. I we would stop implementing other stuff because it might be superseded by something else later on then development would come to a grinding halt immediately. :wink:

By the way, I like your designs very much. :art:

[tresf]

... we also need better widget overflow support (toolbar suffering this now) to really have spillable columns content. Hopefully the ZynGUI2 will help provide some of Qt's missing layout managers.

[Wallacoloo]

So should we merge this then? (after somebody pulls the branch, runs $ rebase master and then tests it again?)

[tresf]

@Wallacoloo I think you and @michaelgregorius know best. I fathom introducing this will make future changes easier anyhow.

[michaelgregorius]

@Wallacoloo I have just rebased and tested against master and now it seems that the LADSAP dialogs also suffer from the problem described in #2288, i.e. they open in a very small state. Another problem (I think it was also present in the original pull request) is that the dialogs have no minimal horizontal size and the layouting in general could still be improved, e.g. when making the dialogs very large.

So currently I would advise against a merge.

[tresf]

open in a very small state

@michaelgregorius this should be fixed via #2358 right?

[michaelgregorius]

@tresf Sorry, I cannot test this at the moment because LMMS (still) cannot find the LADSPA plugins. There was a bug report about this but I cannot find it anymore. To reset everything to a clean slate I have removed my lmmsrc.xml and started LMMS with an installed version (make install) but it doesn't find the plugins with the default paths. It doesn't even find them if I point the LADSPA folder to only use the system wide installed plugins (/usr/lib/ladspa/). Any tips?

[michaelgregorius]

@tresf Ok, I was able to check it. Unfortunately the problem is still there.

[tresf]

@michaelgregorius so the LADSPA issue is fixed, but the dialog sizes are still off?

[Wallacoloo]

@michaelgregorius Make sure you're correctly matching 32/64 bit. Somehow, I have both 32 and 64-bit versions of the ladspa plugins on my system, at /usr/local/lib32 and /usr/local/lib. Point lmms to the appropriate directory if you do too. You can verify the architecture of both lmms and the ladspa plugins by using file ./lmms, etc.

[michaelgregorius]

@tresf Yes, exactly. The LADSPA problem seems to be a build issue when building with Qt Creator. The dialog sizes are still off.

[tresf]

@michaelgregorius bumping to 1.3 milestone. Fine to merge sooner if the rebase and testing occurs. 👍

[michaelgregorius]

@tresf I'm OK with bumping this one to 1.3. Otherwise 1.2 will never make it to a release. :wink: :smile:

[michaelgregorius]

Intermediate rebase against cea7d7d0028bd6aa8ef7d29f1d2f09fc8d57bd53.

[gi0e5b06]

And after one year and half, this excellent work is still not merged...

[tresf]

@gi0e5b06 did you test this? Qt4 or Qt5? It's not working properly for me. (dialogs were small, had to expand them to see contents).