New : open access to SyphonServerConnectionManager and Syphon global …

[mto-anomes]

Hi,

I've been working on a Syphon plugin for Unity and got forced to alter the Syphon framework to make it work properly with the Unity OpenGL context. I thought maybe there's a better way to handle this kind of situations.

This PR gives to developpers the possibility to create new types of Syphon Servers (for Metal, for specific OpenGL contexts, for custom shaders... you name it !) without having to change the framework in any way. It permits to extend the framework with add-ons instead of having to dig inside the framework and make breaking changes.

To do so, this PR gives access to the SyphonServerConnectionManager header and important global variables from SyphonPrivate, so the developper can have access to all the elements needed to handle syphon notifications and server options.

I created a new SyphonConstants class to give access to SyphonPrivate elements (extern/static variables, preprocessors variables) easily without introducing any breaking change to the current code.

I purposely did not add the new headers to the Syphon.h, as this is an advanced feature that could confuse the average user.

Tell me what you think Best, Maxime

[bangnoise]

Interesting idea - I had wondered about this problem. As long as it doesn't break any of the actual work of sharing (ie as long as it correctly interacts with unmodified framework clients) I'm in favour of this idea in principal.

Why do you add the SyphonConstants class? Why not just expose the constants themselves?

Although it would require more dramatic refactoring, it might make sense to add a base class to SyphonServer which would handle creating and resizing the IOSurface and broadcast, and one could make a custom server by subclassing this base class (it would also allow use of an IOSurface of ones own). This sort of change will be necessary for incorporating Metal support in the framework anyway.

I'm going to leave this open and think about it as we consider changes for Metal support - it ties in too closely with that to treat it in isolation.

Don't modify anything now but in future it would be helpful if you could isolate changes which are purely refactoring (such as moving member variables inside the implementation of SyphonServerConnectionManager) into at minimum separate commits or at best separate PRs - it makes it easier to review and quickly incorporate useful refactoring and focus on the important changes.

Thanks!

[bangnoise]

Also it would be interesting to know the original issue you hit with Unity - we'd hoped SyphonServer worked happily with any host GL context, so it would be useful to know if not - open a separate issue for that if you can.

[mto-anomes]

Hi, thanks for your feedback, I highly appreciate it.

I decided to create the SyphonConstants class for two reasons :

• Exposing public variables in a framework header can rapidly become a messy thing to do as these variables will join the global scope of any developper's project using the framework. It seems to me this habit should be avoided when possible. Putting the constants in a class solves this.

• The SyphonPrivate.h constants are at this moment a mix of static variables (extern static) and preprocessor definitions (#define) whom by definition cannot be exposed as they disappear in the compilation process. In my time window I could not figure out the reason of this duality so I prefered to create a separate class rather than alter the SyphonPrivate code. Maybe there is some refactor possible on this matter. What's your opinion about this ? I could work a PR on this.

I agree with your point on the need of a more "dramatic" refactoring. If I may add, it would be awesome to make the base code of Syphon completely agnostic of the graphic engine.

For the issues I ran into with Unity, without going into too much details it has to do with the handling of alpha channels and sRGB framebuffers provided by Unity. I'll open an issue when we're further ahead in development and if it makes sense to do it.

I hear your request for smaller and more granular PRs and will work on it shortly.

[bangnoise]

Closing this PR in favour of ongoing work in the subclassing branch, which should solve the problem in a more useful way.