Restructuring of the CSharp library

[Gallimathias]

I have restructured the CSharp library. Now the lib structure is more like other CSharp projects.

This makes the work on this library much easier.

[dorodnic]

Hi @Gallimathias
Thank you for the pull-request.
We will evaluate the proposed changes and update this thread.

[sebandraos]

I know this isn't part of this PR per se, but why is a c# project being built with cmake? This seems like a duplication of the job the csproj is supposed to be doing in the first place.
Other than that, this PR looks positive to me (not that I have any influence over whether it's accepted)

[dorodnic]

Hi @Gallimathias @sebandraos
I agree this PR improves C# wrapper readability. Unfortunately, it seem to be superseeded by #2559, so some work will need to be done to merge it. Either this PR will get updated or we will do this work and ensure proper attribution.
@sebandraos regarding your comment on CMake - csproj solution would make sense if the wrapper was a standalone thing, but being part of bigger librealsense project it makes perfect sense to me to build it together with rest of the library.

[Gallimathias]

@dorodnic Thanks for the comment. I'm trying to merge #2559 into my branch tonight.

@sebandraos Thanks for your support. Just like you, me and a two buddy were confused about cmake. In my case, i get trouble with the cmake generated sln. But I also understand @dorodnic with his argument.

I think we should actually think about an sln check in.

[matkatz]

@Gallimathias thanks for this PR.
Please notice that we have merged some big changes of the C# wrapper to the development branch (PR #2592). Also, your changes requires upgrade to the C# language version from 6 to 7, that property is set in "librealsense\wrappers\csharp\CMakeLists.txt".

[Gallimathias]

@matkatz Thanks for the hint, I just restarted merging now ;)

I'm hurrying to get the PR up and running as soon as possible.

[Gallimathias]

So after hours I made it. I hope other PR's will be separate. :D

Good night together zzzz

[matkatz]

Thanks again.
The changes in this PR requires to upgrade the C# RealSense wrapper to .Net 4 which is currently limited by the Unity wrapper.
We will try to check the possibility to upgrade to a newer version of Unity that supports .Net 4.

[Gallimathias]

Unity will always slow down.

I suggest compiler statements. So there would be two compiled versions for Unity and for the industry as I need it for example.

But there are supposedly ways to add a newer version to unity.

[Gallimathias]

@dorodnic @matkatz

I did some research on Unity. Since version 2018.1 .NET Standard 2.0 and DotNet 4.x are supported. (Current Unity version: 2018.2)

If you want to support the older versions (.NET 3.5 (Unity< 2018.1) ) Then please let me know and I will include compiler statements in the release. I think this is the most sensible compromise between industry and Unity development.

https://blogs.unity3d.com/2018/03/28/updated-scripting-runtime-in-unity-2018-1-what-does-the-future-hold/

[Gallimathias]

@dorodnic @matkatz

For Unity versions smaller than 2018.1 I added preprocessor instructions (NET 3.5).

I also defused the locks in the pools.

[dorodnic]

Thanks @Gallimathias
We will try to merge before next SDK release

[ogoshen]

First of all, good job with this PR.

Everyone likes sugar with their code, and the fat arrow syntax does more than just that I think, it reduces syntactic noise helping focus on what's important, and can make it easier to draw in more contributors.

Having said that, a prerequisite for these arrow functions to compile with this PR, is support for out var which is C# 7 only.
Actually, since the ErrorMarshaler takes care of throwing exception, those out vars can just be discarded.

Being just sugar, compiled code will still run on older frameworks.

So the question I guess is what compilers and frameworks are we going to support.
Framework support must include .NET 3.5 for Unity (LTS) support as @matkatz stated, which this PR covers.

As for compilers, I vote for ditching everything but the .NET Core SDK.
This kills more birds with one stone:

• C# >=7
• Higher portability
• dotnet CLI (@sebandraos's comment about cmake)
• docfx for generating docs (2706)

Had an issue targeting 3.5 with core sdk, this solved it for me.

[Gallimathias]

Thanks for the compliments.

I have not made any logic changes. As I said there are pure structure changes. (I don't like it when there are too many changes in a PR at once) (Ok I made the pooling threadsafe)

If you ignore CMake like other C# developers, the wrapper will compile to the .NET standard. Which should be the goal by now. If you don't have csproj and sln checked in you can compile the project as you like.

Currently I always keep the PR up to date. It is a pity that it is not merged because I still have some todos for the wrapper on my list which I would like to contribute.

I'm currently thinking about two posibilitys. One: opening more PR's that have this PR as their origin or Two: I'm create a hard fork of the c# wrapper.

[ogoshen]

.NET Standard is the way to go I agree, and hang in there with the PR it could take a while...

I suggest you share your plans in this discussion or as new feature request issues if you deem them important.

BTW, pooling was already thread safe, ConcurrentStack makes it lock-free with atomics.
Since most of the time you'll use the library's thread-safe objects like FrameQueue and dispose of frames properly. The lock is actually rarely used, like when you forget to dispose and the GC thread kicks in... ConcurrentStack when available is still preferable.

[dorodnic]

I'd be happy to merge this given an approval from either @ogoshen or @matkatz, as I'm not sufficiently fluent in .NET myself.
@Gallimathias sorry this is taking so long

[Gallimathias]

@matkatz @ogoshen How much longer do you need?

[dorodnic]

If I correctly follow the discussion:

a. Transition to .NET core requires further evaluation. Getting away from CMake would be a big downgrade IMHO. On the other hand, @matkatz did manage to build the wrapper with .NET core on Linux (but not the WPF examples), which it is nice.

b. File structure - each class in a separate file, better naming and style, etc.. - I believe agreed by all that's a good thing for the project.

c. Language bump to C# 7 - we need the C# wrapper to be functional with .NET 3.5 to enable Unity, and I prefer to keep support for Visual Studio 2015. I might be mixing up the framework / language / VS versions, but this is something that could prevent a merge.

@Gallimathias - perhaps you could create a PR with just (b)? I don't see a reason not to merge it.
Then it will also be easier to manage (a) and (c)

[Gallimathias]

@dorodnic You didn't take some things right out of the discussion:

a) No evaluation to the .NET standard is required as the library is already .NET standard compatible. I have compiled the library several times as .NET standard. The discussion was only about checking in the sln because cmake makes no sense here at C#. But the sln and cmake are not mutually exclusive.

WPF will never run on Linux, (at least not for a while) because WPF uses DirectX which is not available on Linux.

b) Well, that's a relief for everyone.

c) This point tends to conflict with a).

We don't need a separate wrapper for 3.5 - that's nonsense, there are corresponding tools in C# for that. These two lines of code for the compiler instructions are built in.

Conclusion: You want a PR only for point B. I've been making it available the whole time because that's exactly what this PR is, which is why I don't understand the whole discussion.

Yes, the compiler statment is already available here (that means the PR is 3.5 capable).

I made the fix because of the thread security which was not considered with the Pool PR in the assumption.

The pool error has already been mentioned in PR #2810 and the fix there was only moderately viewed because the fix was also only half-hearted.

That is by the way quite annoying for not hobby projects.

[Gallimathias]

Dear @dorodnic and @ev-mp,

That's a little disrespectful, don't you think?

I reinvested quite a lot of time and took part in the discussion. My questions were ignored.

Now @matkatz simply copies my code, he doesn't even merge my repo and adjusts things. No he COPERS.

And that will be merged without a discussion without an ifs or buts. I have neither the Contribute flag nor a thank you.

It is a pity how you deal with "voluntary" work from other people. And that's not the way Open Source works. That's just disrespectful. By the way, I can't just copy Intel code without mentioning Intel.

Or do you think that this is the new way of using work by third parties?

Even if there is little creative work involved in restructuring. It was work, it took me time and patience. And if you have work from third parties you should never, never, never step on them.

I hope you can read from this that I'm pissed how you've been handling my time.

And I hope for others that they will be treated more respectfully.

[dorodnic]

Hi @Gallimathias
We mean no disrespect.
This PR cannot be merged, because it breaks .NET build.
Since you didn't want to cherry-pick file structure only, as I suggested in (b), @matkatz redid this work himself, without changing C# version.
You are credited in both the commit message and the PR.