Cycling74 / min-api

High-level C++-language application programming interface for Max
MIT License
58 stars 23 forks source link

Remove one class per DLL limitation #114

Open tap opened 6 years ago

tap commented 6 years ago

min::this_class etc. is a problem.

Statically defined in the namespace, so there’s only one class per DLL

pixsperdavid commented 4 years ago

If this issue is resolved, are there any other barriers to having multiple objects defined in a single DLL? I'm running into an issue regarding multiple object DLLs referencing the same static library, whereby they will not function correctly unless compiled against the MSVC DLL runtime (which places a burden on the user to install it).

tap commented 4 years ago

@impsnldavid -- is it possible for you to provide a more concrete example?

I don't think there should be other barriers, but I'd also like to better understand the problem you are referencing.

pixsperdavid commented 4 years ago

@tap My LXMax project is structured as multiple object dlls and an extension dll, all of which reference a central static library with shared code.

If compiled against the MSVC MT/MTd runtime, then when accessing the shared resources I get a whole heap of runtime errors, which after some research I discovered is because (roughly speaking) the static MSVC runtime code generated is not guaranteed to be compatible between different targets. The problems occur when an object gets passed an instance of a shared class which was created by another object. The same objects compiled against MSVC MD/MDd work correctly.

The other solution to this problem would be to make my central shared code library a DLL, however this would entail a whole heap of extra work to avoid passing stl classes which I would like to avoid. Seeing as there is no requirement other than Max convention for the objects being in separate DLLs, it seems to me that the best solution might be to make the whole set of objects part of the same assembly. This would also remove a lot of potential user pain points as the design of the library prevents use of any of the objects unless all the versions match exactly.

tap commented 4 years ago

Thanks for providing the clear explanation and additional info @impsnldavid. Yes, you definitely do not want to pass STL data across DLL boundaries.

Have you done (or are you preparing) any work on this issue? If so, I'm happy to prioritize any review or brainstorming etc. with you!

pixsperdavid commented 4 years ago

@tap Not yet done any work on the issue, just wanted to check that there wasn't a deeper max-api related reason that might restrict classes to one per dll! Happy to look into resolving the issue though, will take a look at the relevant parts of the min-api and see what needs to change.

pixsperdavid commented 4 years ago

I've done an exploratory conversion of my library to package all external classes into a single DLL which builds successfully and is loaded by Max. None of the externals are available for creation, so will now need to investigate what is happening during the wrapping process, however this puzzles me as I was expecting to at least see some crashes or console errors.

tap commented 4 years ago

Hey David, feel free to send to me privately if you want another set of eyes on it.

Cheers!

pixsperdavid commented 4 years ago

https://github.com/impsnldavid/lxmax/tree/single_dll_conversion

I've published the changes I made to a new branch above.

pixsperdavid commented 4 years ago

Had a bit more time to spend on this finally, it seems that Max's behaviour in loading externals doesn't allow what I'm trying to do. It seems that Max uses the filename of a DLL to determine the object name (which makes sense as if it does not call ext_main until the external object is first instantiated then it has no way to determine the actual object name). I tested this by renaming an external DLL and saw the new name appear in the autocomplete list.

So therefore, if Max uses the DLL filename to determine the object name, how can it ever be possible to bundle multiple objects into a single DLL?

grrrwaaa commented 4 years ago

If you put an .mxo/mxe etc. into the /extensions folder of a package, then Max will load & bind them at startup. The ext_main here should be able to register multiple class names in this case.

On Sep 26, 2020, at 1:20 PM, David Butler notifications@github.com wrote:

Had a bit more time to spend on this finally, it seems that Max's behaviour in loading externals doesn't allow what I'm trying to do. It seems that Max uses the filename of a DLL to determine the object name (which makes sense as if it does not call ext_main until the external object is first instantiated then it has no way to determine the actual object name). I tested this by renaming an external DLL and saw the new name appear in the autocomplete list.

So therefore, if Max uses the DLL filename to determine the object name, how can it ever be possible to bundle multiple objects into a single DLL?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

pixsperdavid commented 4 years ago

Thanks @grrrwaaa, I had tried this before and not seen my externals registered, however have now realised this was due to the min-sdk not removing the .hpp extension when deducing the max class name! I will update this as part of my forthcoming pull request.

pixsperdavid commented 4 years ago

I have an initial proposal for changes to resolve the use of min::this_class, please see the commit below for changes:

https://github.com/impsnldavid/min-api/commit/33dda6ef2ef769c47dab4a48c3a10fbf36698b01

To summarise, I have removed the 'this_class' static variables and replaced them with a templated struct containing the same variables. This allows for a separate set of variables per min class type. I've also added an 'all_classes_dummy_constructed' variable which is set to true as part of ext_main. This is used in the 'error' function to prevent it having to be templated.

The only issue I can see remaining with this approach is the use of 'this_class' in the set method of attribute (https://github.com/Cycling74/min-api/blob/master/include/c74_min_attribute.h, line 613). One option would be to add a min_class_type template parameter to attribute, but this would affect a lot of existing code. It doesn't seem that the set function needs to access the instance, rather just check that it is not nullptr. Perhaps there is an alternative way of doing this? Thoughts?