On Wed, 27 Nov 2024 17:04:27 +0100
ichthyo <p...@ichthyostega.de> wrote:
>On 27.11.24 13:01, Will Godfrey wrote:
>> It seems we have two quite different mechanisms for identifying this.
>> The first is the compile time YOSHIMI_LV2_PLUGIN
>>
>> The other is a runtime one, runtime.isLV2
>
>Indeed I noticed that too during my clean-up.
>However, it is more complicated than that.
>
>In the old version, this distinction was hidden somewhat confusingly
>and came to effect as a combination of the info that was previously
>encoded into the "lv2pluginType" in the Synth engine. It was passed
>through several other flags, and I traced them down and found out
>that in the end it maps down into two distinct behaviour switches:
>
>isLV2
>isMultiFeed
>
>(isMultiFeed is also activated by the Jack engine)
>
>I thus factored this out of the SynthEngine and consolidated those flags
>into the Config, where they belong IMHO.
>
>
>
>Now the question is: is it really better if we introduce yet more
>preprocessor magic and conditional compilation? I can understand that
>sometimes one is forced to exclude something from compilation because
>a library is not linked in and a call would thus not compile, e.g. when
>building without GUI.
>
>But doing this kind of compilation switches is always a compromise and
>makes the code harder to read and maintain.
>
>
>Just to quote some obvious example:
>
> bool inSync = runtime().isLV2
> or (runtime().audioEngine == jack_audio
> and runtime().midiEngine == jack_midi);
>
>
>Nice and clear to read.
>Now consider what you need to do, if you want to handle
>one case distinction with the preprocessor, and the other
>one at run time. You would not gain anything, but the code
>will be less clear to understand.
>
>
>Another example.
>This happens when the LV2 plugin load completes successfully:
>
> // Configuration for LV2 mode...
> runtime().isLV2 = true;
> runtime().isMultiFeed = isMultiFeed(lv2Desc);
> synth.setBPMAccurate(true);
>
>
>Now again: would it improve the code if one of these settings (isLV2)
>is hidden from this block and instead done at a completely different
>code location, by some conditional compilation magic?
>
>
>Then add to this, that you would have to use conditional compilation
>at several places in the GUI code, and ensure that it gets properly
>through the FLTK generation process. Not really fun and would
>also make the code less clear and more brittle IMHO.
>
>For example (Note: this is FLTK generated code....):
>
>void ConfigUI::Show(SynthEngine *synth) {
> //
> if (isLV2() == false and synth->getUniqueId() == 0)
> singleMaster->show();
> int tmp = fetchData (0,
> CONFIG::control::alsaMidiType,TOPLEVEL::section::config);
> alsaType->value(tmp);
> if (tmp == 0)
> alsaSource->activate();
> else
> alsaSource->deactivate();
>....
>
>
>So personally I can see no real improvement when replacing
>actual code by preprocessor switches, other than saving a small number
>of assembly instructions in the binary.
>
>
>Maybe we could even ask the opposite question: is there a deeper reason
>why we have to re-build the whole code base a second time, just to create
>the LV2 plugin? Wouldn't it be sufficient to build one static library
>and then just link it in the final step either with main.cpp or
>with some additional stubs and generate the .SO for the plugin?
>
>Maybe it is just the way it is because it was challenging to get
>CMake to work and do what we want on all platforms...
>
>;-)
>
>-- Hermann
Thanks for that Hermann - a nicely detailed analysis :)
Maybe we should go the other way and replace the preprocessor switches.
This might seem unnecessary, but to me, having two different systems
independently controlling the same thing raises a red flag.
--
Will J Godfrey
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
