dotnet / Silk.NET

The high-speed OpenGL, OpenCL, OpenAL, OpenXR, GLFW, SDL, Vulkan, Assimp, WebGPU, and DirectX bindings library your mother warned you about.
https://dotnet.github.io/Silk.NET
MIT License
4.19k stars 406 forks source link

Extensions do not work on full trimming (audit Activator usage) #2010

Closed Perksey closed 7 months ago

Perksey commented 7 months ago

Easy issue. May need some linking attributes littered some places. Reported in Discord.

alexrp commented 7 months ago

Took a quick peek. The vast majority of cases just need [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] on the type parameter and they should be good.

Cases like this, though, will never be trimming-compatible, I don't think:

https://github.com/dotnet/Silk.NET/blob/3870647d8af2b79db0ff9290472305d2c096cf90/src/Input/Silk.NET.Input.Common/InputWindowExtensions.cs#L99-L122

Unless we can somehow abuse InputPlatformAttribute to signal to the linker to keep the type... :thinking:

alexrp commented 7 months ago

Since we build for TFMs that don't have [DynamicallyAccessedMembers], would you prefer to pull in PolySharp, or use #ifs?

Perksey commented 7 months ago

Yeah the paths you linked are only used in non-AOT scenarios - users are already expected to use GlfwWindowing.Use and friends to register the platforms today on AOT.

I agree with your assessment as well, it seemed like this was just a case of adding attributes. It would be nice if we didn’t need to use the Activator at all, which in theory could be possible using static abstracts in interfaces but only on newer platforms, so I guess we can deal with it for now.

So in any case we’ll need to add some TFM hackery. We probably shouldn’t pull in PolySharp as we try to avoid pulling in non-.NET Foundation packages in core projects. Recommend an #ifdef or something similar to how I handled RequiresLocationAttribute.

Perksey commented 7 months ago

Modified the CoreRTTest project to add a TryGetExtension call with PublishAot=true, PublishTrimmed=true, and TrimMode=full. Fixed with the changes in #2115