danielscherzer / GLSL

VSIX Project that provides GLSL language integration.
257 stars 28 forks source link

[Feedback Request]Added Support for VS2022 #86

Closed TheEndHunter closed 3 years ago

TheEndHunter commented 3 years ago

I have created a fork of the repo Here and made a branch with the feature added as a preview if people wish to check it out first and see if it is a good fit for a pull request. it builds with no issues and can be installed(Marked as preview since the Vs2022 nugets are previews) and I am in the process of testing it so feedback is appreciated.

A few other changes I made are updating to .net4.8 and cleaning up some of the message/warnings from the complier output.

[edit 22/08/2021] I Have Since split the project into 3 to support still having vs2017 along side vs2019 in there own sub project, and vs2022 as it's own project, see the pull request Here

other than those changes the code is identical to the previous versions.

danielscherzer commented 3 years ago

Thanks for your contribution to the extension! I downloaded your Feature/VS2022_Preview branch, compiled it (I only have VS 2019 available for now) with some warnings about package conflicts successfully. I start debugging in the experimental instance. As soon as I load a shader I get the following exception:

System.NotSupportedException
  HResult=0x80131515
  Message=The currently bound window platform(s) only support views, instead of windows. Use the view APIs instead.
  Source=Silk.NET.Windowing.Common
  StackTrace:
   at Silk.NET.Windowing.Window.Create(WindowOptions options)
   at DMS.GLSL.Errors.ShaderCompiler.CompileOnGPU(String shaderCode, String shaderType, ILogger logger) in D:\temp\gits\GLSL\src\Errors\ShaderCompiler.cs:line 218
   at DMS.GLSL.Errors.ShaderCompiler.Compile(String shaderCode, String shaderContentType, ILogger logger, ICompilerSettings settings) in D:\temp\gits\GLSL\src\Errors\ShaderCompiler.cs:line 167
   at DMS.GLSL.Errors.ShaderCompiler.TaskGlAction() in D:\temp\gits\GLSL\src\Errors\ShaderCompiler.cs:line 86
   at System.Threading.Tasks.Task.InnerInvoke()
   at System.Threading.Tasks.Task.Execute()
TheEndHunter commented 3 years ago

Fixed, Just Releasing a Preview Build Via Appveyor now, reverted to OpenTK and it fixed the issues, from what I found silk.net copies some binaries for specific platforms at build over but it appears it isn't doing it for this build, so I'm going to investigate deeper into the issue on another branch.

danielscherzer commented 3 years ago

Earlier I had reports of VS 2017 having trouble with the newer Microsoft.VisualStudio.SDK packages. So I avoided merging your pull request. Instead I did a very small update, only changing the manifest and automatic code cleanup. Chould you please check if this minimal change is enough to run the extension on VS2022.

danielscherzer commented 3 years ago

Update: Microsoft does not allow me to publish this version of the extension on the marketplace. Mirgration seems to be more involved (https://docs.microsoft.com/en-us/visualstudio/extensibility/migration/update-visual-studio-extension?view=vs-2022) and seems to break support for VS 2017. VS2022 seems to be amd64 only. I will check if VS2017 support ist still something we need.

TheEndHunter commented 3 years ago

Update: Microsoft does not allow me to publish this version of the extension on the marketplace. Mirgration seems to be more involved (https://docs.microsoft.com/en-us/visualstudio/extensibility/migration/update-visual-studio-extension?view=vs-2022) and seems to break support for VS 2017. VS2022 seems to be amd64 only. I will check if VS2017 support ist still something we need.

Would an option be then to just build a 32-bit binary for the vs2019 and lower versions, and then build a 64 bit one separately for Vs2022, the simplest option I can think of without adding complexity to it is make the current project file a shared project for 2 new projects that contain different vsix files, one 64 bit and one 32, then went the build is called just make sure that the files end up with the architecture appended to the vsix's generated. Let me know if you want me to mock it up in a new branch on my fork.

TheEndHunter commented 3 years ago

Earlier I had reports of VS 2017 having trouble with the newer Microsoft.VisualStudio.SDK packages. So I avoided merging your pull request. Instead I did a very small update, only changing the manifest and automatic code cleanup. Chould you please check if this minimal change is enough to run the extension on VS2022.

From what I got off the merge it has built fine, so I shall try installing and having a play around on vs2022 experiential instance when I'm next at my pc. But given that to build for Vs 2022 it needs to be 64 bit in the vsix, my previous suggestion may work as a better alternative as it means that the code can remain the same and it can be built for both architectures.

danielscherzer commented 3 years ago

I did a package update of Microsoft.VisualStudio.SDK in the master branch and on the VS marketplace to check if any user would report problems with this update alone. I'm waiting for confirmation from #89 but it seems like support for VS2017 will be troublesome.

TheEndHunter commented 3 years ago

I did a package update of Microsoft.VisualStudio.SDK in the master branch and on the VS marketplace to check if any user would report problems with this update alone. I'm waiting for confirmation from #89 but it seems like support for VS2017 will be troublesome.

I Have a new branch to investigate a possible solution its just having some issues with the vsix, Here I Have Split the Project into 3 Sections, a Shared Project with all the code, and a 2 Architecture Specific VSIX projects. the building is set up so that it build both architectures at once, the current issue I have is with the x86 project not building due to some validation issues, but I'm in the process of investigating a possible fix. if you like the idea let me know and ill merge it into my master branch so that when the pull requests is accepted it should then merge fine.

TheEndHunter commented 3 years ago

Closing this as feedback on this merge is no longer needed as it does what I planned for it, supporting the installation of the extension onto vs2022.