MikePopoloski / SharpBgfx

C# bindings for the bgfx graphics library
MIT License
169 stars 32 forks source link

Convert SharpBgfx to .NET Core #3

Closed benpye closed 7 years ago

benpye commented 8 years ago

This converts SharpBgfx to use an xproj project, and targets dotnet and net45 so we keep compatibility with existing targets. This probably shouldn't be merged as it's currently using the functionality suitable for the latest version of Visual Studio.

This adds a SuppressUnmanagedCodeSecurityAttribute dummy class for .NET Core where it isn't available (it's default there). I haven't yet done the examples, but these shouldn't be too hard. They could still be just net45 targets anyway. This change also makes it very easy to publish a NuGet package with the dotnet cli.

I have a couple issues with the code space more generally that may cause issues if more targets are to be supported.

  1. The string marshalling is probably wrong. Currently ANSI is used, but I would expect Bgfx to use UTF8. This is fine on non Windows platforms where ANSI actually means UTF8 (on coreclr anyway).
  2. Not a great fan of changing the dll name invoked based upon debug or non debug builds. It would be possible that you would want to use a release bgfx library always, even with a debug binding or the inverse. This is also a little unpredictable if it's to ever be published as a NuGet package.
MikePopoloski commented 8 years ago

Nice. It's been a while since I looked at the .NET Core stuff; they keep changing around how they want to set up projects for it.

I actually don't think bgfx supports UTF8 specifically. In most cases it delegates string manipulation to the C standard library, and on Windows at least that's definitely not going to play nice with UTF8. There's not much string handling anyway; it's mostly all for debugging info.

As far as debug builds go, for the most part that would not be an issue with deployed binaries because you'd only ship a release build of SharpBgfx... there's little reason to ship a debug one I think. There are good reasons to want a debug build of bgfx though, as sometimes things go wrong and you want to step the source. If you have a better suggestion I'm open to it.

benpye commented 8 years ago

Ah okay, I think I expected UTF8 since that's also what OpenGL uses for example, but if it converts to the system native encoding, then it's likely ANSI or UTF16 (on Windows and OS X).

For most libraries I have seen it's a case of switching out the binary. Additionally .NET Core exposes AssemblyLoadContext which allows the application to decide the unmanaged binary to be loaded (based upon the name given to DllImport), but unfortunately this isn't supported in .NET before 4.6 I believe. This would let the consumer of the library make this choice rather than the library itself. I just think it could be somewhat surprising that the library is loading a different binary if it's in debug mode or not.

klaussilveira commented 8 years ago

Any news on this one?

MikePopoloski commented 8 years ago

I've been waiting for .NET Core to finalize how they approach project management before i reorganize everything. It seems like they're still not sure what the tooling around all of that will look like.

benpye commented 8 years ago

Unfortunately tooling for .NET Core isn't going to RTM until after VS 15 releases, the libraries etc should be stable as of RC2 though. See https://blogs.msdn.microsoft.com/dotnet/2016/05/06/net-core-rc2-improvements-schedule-and-roadmap/

xgalaxy commented 7 years ago

Yea. I'd say as soon as they drop the json project format and get all the cli tooling on csproj would be a good time to start looking at this.

MikePopoloski commented 7 years ago

I committed a change this morning to convert everything to the VS 2017 .NET Core tooling system.