Skip to content

Conversation

DmitryGolubenkov
Copy link
Contributor

@DmitryGolubenkov DmitryGolubenkov commented Sep 6, 2021

Summary of the PR

  • Added a net6.0 target to Silk.NET.Core.csproj
  • Added a new unsafe struct NativeMemoryPointer inside a preprocessor NET6_0_OR_GREATER
  • Modified GlobalMemory.Free and GlobalMemory.Allocate to use the new struct.

Related issues, Discord discussions, or proposals

#597

Further Comments

I don't know if this PR require testing or contain breaking changes as I am not aware of any possible problems.

I had to add unsafe to GlobalMemory.Free to use NativeMemory because that is required. And this change will affect .NET 5 and .NET Standart versions even if this change is only for .NET 6.
Also: maybe it is better to use NET6_0_OR_GREATER directive in GlobalMemory.Free? Because NET6_0 targets only one version, and I think next year someone will have to do this change anyway.

NativeMemoryPtr requires minimum .NET 6 too. Also I had to make some convertions line (nint) and (nuint) to be able to use NativeMemory.

I think that GlobalMemory.Allocate requires a little work, because the preprocesor directives now are.. Confusing.
Now it is:
#if NET6_0
#elif !NET5_0
#else

I think it is better to make
#if NET6_0
#elif NET5_0
#else
and make according changes to the code. Only to make the code more readable.

@HurricanKai
Copy link
Member

Thank you for you contribution!
I agree that the preprocessors in Allocate are confusing, if possible do change that.
Also, there are a lot of places in Silk.NET.Core where NET5_0 is used, and those codepaths should be available on NET6_OR_HIGHER as well, if possible please change those as well.

@DmitryGolubenkov
Copy link
Contributor Author

Should I make another PR to make these changes to whole Silk.NET.Core? Or just commit everything to this?

@HurricanKai
Copy link
Member

Committing to this is fine

@DmitryGolubenkov DmitryGolubenkov marked this pull request as ready for review September 6, 2021 15:47
@Perksey Perksey enabled auto-merge (squash) September 6, 2021 15:56
@Perksey
Copy link
Member

Perksey commented Sep 6, 2021

Thanks again for the shear volume of contributions as of late! Really appreciate it :)

@Perksey Perksey merged commit 04ae5ed into dotnet:main Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants