Skip to content
This repository has been archived by the owner on Mar 30, 2019. It is now read-only.

Updated to .NET 4.5.2 and to work with SharpDX 3.0.0 #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SmartK8
Copy link

@SmartK8 SmartK8 commented Feb 13, 2016

Hi, nobody was raising to a challenge so I did my humble fork. These are some changes I did:

  1. Interop - I basically cloned the auto-generated unit from SharpDX (named it Native) and added another class to SharpCLI to process. Therefore Fixed method is not faked and is fast as was before.
  2. Shader stages - I was trying to solve how to deal with classes instead of IntPtr then I gave up and basically I'm calling (now internal) methods in SharpDX via reflection. Therefore nothing changed except it does reflection (all those MethodInfos can be cached probably).
  3. WP8 - Removed from building scheme - because I can't imagine how it even work with new SharpDX
  4. Mono.CCecil - I updated to 0.9.6.1 to be able to utilize autogeneration from 1)
  5. Misc - Quite few basically cosmetic changes there were quite straightforward (ex. SetInputLayer to InputLayer = value, etc.) and also some things that just happened (auto update of some packages) and minor details.

Still remains: Fonts were not visible in my game, I still need to determine whether it's caused by SharpDX or something trivial.

This is my first fork, first pull request, first time with GIT. I hope I didn't messed up something. It just I really need SharpDX Toolkit to be up-to-date. It works with my quite complex project (2D isometric game in early stages). I didn't noticed any slow downs or problems, but I don't use all of the things from Toolkit so let me know.

@SmartK8
Copy link
Author

SmartK8 commented Feb 13, 2016

OK, I fixed that issue with fonts. It was my mistake. Everything is running for me now as it did before in version 2.6.x. No changes required in end-project (except for new Toolkit assemblies obviously).

Update: There's a link to download binaries on my fork Wiki (only one page). You can try it out.

Download binaries (on DropBox)

@xoofx
Copy link
Member

xoofx commented Feb 14, 2016

Someone as already started a similar work #3, Ideally I would prefer If you could join the effort with @tomba I guess that you share basically the same goals?!

  1. Not sure to follow exactly, but having either SharpDX as a submodule of the Toolkit, and referencing directly the SharpCli csproj from there could do the trick (and modify Toolkit target files accordingly)
  2. I don't mind to make this methods public in SharpDX directly.
  3. we can remove support for WP8
  4. same as 1) if sub-module
  5. ok

@SmartK8
Copy link
Author

SmartK8 commented Feb 14, 2016

Hi, thank you for your response. #3 was exactly the reason I started to make modifications, because 1) it's not a pull request by its own admission, 2) it uses Pin instead of Fixed, 3) it didn't solve the main problem ("solved" with calling internal methods) Toolkit had.

  1. Toolkit project files (csproj) already contain SharpDX.PreSettings.targets (and more noteably SharpDX.PostSettings.targets which contains SharpCLI task). After I cloned repository, Source\Tools folder already contained SharpCLI, so the only thing needed was to update Mono.CCecil (to work with .NET 4.5.2) and update line:

if (method.DeclaringType.Name == "Interop" || method.DeclaringType.Name == "Native")

To allow processing of new Native class (copy of now internal Interop). So I agree it (Fixed) requires almost no change.

  1. That would be better I guess, because this is the main reason Toolkit is not compatible (buildable) out-of-the-box. But I can see how the public API looks cleaner without those IntPtrs. :)

  2. I agree submodule would be great solution (this implies changing projects paths to SharpDX projects wherever they'll end-up as a submodule). I was forced to unload SharpDX projects because I don't have SNK, and I wanted to be sure my build of Toolkit will run against 'official' SharpDX build. I can imagine having two sln files (one without SharpDX project references).

New Toolkit will probably lack many of new features (DirectX12), but those can be access directly via SharpDX if one must. I'm using parts of Toolkit, because it contains many classes I would be forced to write anyway without it (I'm talking about hundred of them). I believe many people using Toolkit will be in a similar predicament. So it's cheaper for me to maintain whole Toolkit than split it to separate library with a possible rewrite to better suit my needs. Just to illustrate my motivations a bit.

@tomba
Copy link

tomba commented Feb 14, 2016

Huge commits combined with re-indenting the files make these quite impossible to review.

Anyway, one of the motivations with my changes was that Toolkit should be just another library using SharpDX. If Toolkit can't be implemented without pulling in internal SharpDX code, using reflection, etc., how can we expect other users to use SharpDX efficiently...

@SmartK8
Copy link
Author

SmartK8 commented Feb 14, 2016

I don't understand the csproj diffs either. It's caused by just opening (or saving) it in VS2015 I think. But when I look at the indentation it seems to be the same (might be tab indentation though). I don't see that much difference to warrant for a all red/green diff. There were not that much changes and they're usually quite distinct. All the csproj should be basically the same.

About the internal methods. The problem is that Toolkit is built with certain logic in mind and it is based on pointers and methods using pointers in SharpDX. People can use SharpDX quite well (better even) if they know from the beginning that they'll be using classes (not pointers). For Toolkit this would mean to rewrite it or at least a very large refactoring.

@RobJellinghaus
Copy link

RobJellinghaus commented Sep 11, 2016

tomba, xoofx already said he would be fine with making those internal methods public. So I don't see why that should be any kind of issue. I will happily submit a PR to SharpDX itself to make those methods public.

I pulled this and now I see the situation. It looks like the only change to SharpDX itself is the Mapping.xml changes. The internal methods called via reflection are:

  • CommonShaderStage.SetConstantBuffers
  • CommonShaderStage.SetSamplers
  • CommonShaderStage.SetShaderResources
  • CommonShaderStage.SetUnorderedAccessViews
  • RasterizerStage.SetViewports

xoofx, would you accept a PR for SharpDX itself, to make those methods public and bring over the Mapping.xml change from this PR?

SmartK8, would you accept a PR for your repository that drops the SharpDX project references and replaces them with assembly references? I am going to try that locally to see if I can get your toolkit repo building against the local SharpDX 3.0.2 assemblies I just built.

I have pulled your PR and I went ahead and dropped the source project references, replacing them with binary references to my SharpDX build. I then went and added public methods for the internal methods above. I thought I could just make them public directly, but it turns out the base internal methods are generated. So instead I created new public methods by appending "IntPtr" -- e.g. CommonShaderStage.SetConstantBuffersIntPtr and so on.

Now I am down to only these compile errors in SmartK8/Toolkit:

Severity    Code    Description Project File    Line    Suppression State
Error   CS2001  Source file 'C:\Git\SmartK8-Toolkit\Source\Toolkit\SharpDX.Toolkit\Native.cs' could not be found.   SharpDX.Toolkit C:\Git\SmartK8-Toolkit\Source\Toolkit\SharpDX.Toolkit\CSC   1   Active
Error       Application Configuration file "app.config" is invalid. Could not find file 'C:\Git\SmartK8-Toolkit\Source\Toolkit\tkfxc\app.config'.   tkfxc   C:\Git\SmartK8-Toolkit\Source\Toolkit\tkfxc\app.config      
Error   CS0246  The type or namespace name 'INotifyPropertyChanged' could not be found (are you missing a using directive or an assembly reference?)    SharpDX.Toolkit.Game    C:\Git\SmartK8-Toolkit\Source\Toolkit\SharpDX.Toolkit.Game\GameWindowRenderer.cs    59  Active
Error       Application Configuration file "app.config" is invalid. Could not find file 'C:\Git\SmartK8-Toolkit\Source\Toolkit\tkfont\app.config'.  tkfont  C:\Git\SmartK8-Toolkit\Source\Toolkit\tkfont\app.config     
Error       Application Configuration file "app.config" is invalid. Could not find file 'C:\Git\SmartK8-Toolkit\Source\Toolkit\tkmodel\app.config'. tkmodel C:\Git\SmartK8-Toolkit\Source\Toolkit\tkmodel\app.config        

The INotifyPropertyChanged error is really the weirdest one in the list. There is no such type anywhere in SharpDX or the Toolkit, and it seems to be a standard type in System.ComponentModel, so I have no idea why the toolkit is complaining about it when trying to call the base GameSystem constructor.

Native.cs is indeed missing from the repository. And I am not sure what is up with all the app.config complaints.

SmartK8, are you still out there? Here's hoping :-)

@SmartK8
Copy link
Author

SmartK8 commented Sep 11, 2016

Hi, first of all.. I'm still here ;)

  1. This is my first attempt to work with GitHub, so excuse my mistake. I somehow missed the files that were added. Still not sure why, because they were still in the commit changes.
  2. I will accept any changes that will better this PR. Removing project references seems OK to me. I have whole SharpDX build environment here. I only wanted to build 3.0.0.0 for myself, but then I decided to give it up, for anyone interested.
  3. By the way the new SharpDX Toolkit 3.0.0.0 is working fine so far (almost half a year). That's good in my book. I can continue my project while using new SharpDX.
  4. I agree that it would be great to have internal methods public again to avoid reflection, but in a meanwhile it works fine.
  5. I don't understand the INotifyPropertyChanged either. It should work. Let's just hope it's an artifact error caused by those other ones. If it's not I would check references/version of FW your project is using. I have it set to 4.5.2. If you have 4.5 (or on the other hand >4.5.2) it could be the difference.

regards,
Kate

@RobJellinghaus
Copy link

Sorry for delay in replying! I am juggling work on an FNA port of my app with work on the SharpDX toolkit. My first attempt at FNA ran smack into a threading issue, so now I want to put some more time in on SharpDX :-)

It is great you are still here, thanks very much for your helpful reply.

  1. I've pulled your change and it got me past the previous issues.
  2. OK, thanks. I am definitely trying to remove anything I can and take a binary dependency only on SharpDX.
  3. That's fantastic to hear. I will spend some more time on this to see if I can get to a similar point.
  4. My local repo has new public methods for these and no longer uses reflection, so I've taken a whack at that already.
  5. The System.ComponentModel problem was simply a missing reference to the System assembly from SharpDX.Toolkit.

I am making more progress, but I have encountered another issue you may be able to help with. At line 307 of Source/Toolkit/SharpDX.Toolkit.Graphics/EffectPass.cs the current code is:

                        // NOTE SmartK8 : Calls internal method
                        MethodInfo setUnorderedAccessViews = shaderStage.GetType().GetMethod("SetUnorderedAccessViews", BindingFlags.NonPublic | BindingFlags.Instance);
                        setUnorderedAccessViews.Invoke(shaderStage, new Object[] { pLinks->SlotIndex, pLinks->SlotCount, graphicsDevice.ResetSlotsPointers });

I modified this to add a new SetUnorderedAccessViewsIntPtr method, called as such:

                       shaderStage.SetUnorderedAccessViewsIntPtr(pLinks->SlotIndex, pLinks->SlotCount, graphicsDevice.ResetSlotsPointers);

However, this does not compile, because the underlying method actually takes four arguments. From sharpdx\source\sharpdx.direct3d11\devicecontext.commonshaderstage.cs near the end of the CommonShaderStage class:

        internal abstract void SetUnorderedAccessViews(int startSlot, int numBuffers, IntPtr unorderedAccessBuffer, IntPtr uavCount);

        public void SetUnorderedAccessViewsIntPtr(int startSlot, int numBuffers, IntPtr unorderedAccessBuffer, IntPtr uavCount)
        {
            SetUnorderedAccessViews(startSlot, numBuffers, unorderedAccessBuffer, uavCount);
        }

The internal method was the one previously being called by reflection, though -- it appears -- with one argument too few. So my question is: what would happen if a reflective call had one argument too few? Would .NET reflection fill in a null for the final IntPtr argument?

For now I am simply passing pLinks->UavInitialCount as the final argument; we will see how far that gets me :-)

Similarly, these lines:

                        // NOTE SmartK8 : Calls internal method
                        MethodInfo setUnorderedAccessViews = mergerStage.GetType().GetMethod("SetUnorderedAccessViews", BindingFlags.NonPublic | BindingFlags.Instance);
                        setUnorderedAccessViews.Invoke(mergerStage, new Object[] { pLinks->SlotIndex, pLinks->SlotCount, graphicsDevice.ResetSlotsPointers, pLinks->UavInitialCount });

I can find no method on OutputMergerStage named "SetUnorderedAccessViews" with the signature (int, int, IntPtr, IntPtr). The closest method I can find is:

internal void SetUnorderedAccessViewsKeepRTV(int startSlot, int numBuffers, IntPtr unorderedAccessBuffer, IntPtr uavCount)

Is that the correct method? For now I am commenting out these calls to mergerStage.SetUnorderedAccessViews.

And finally it looks like there is just one more gosh darn missing app.config file:

4>C:\Git\SmartK8-Toolkit\Source\Tests\SharpDX.Toolkit.Graphics.Tests\app.config : error MSB3249: Application Configuration file "app.config" is invalid. Could not find file 'C:\Git\SmartK8-Toolkit\Source\Tests\SharpDX.Toolkit.Graphics.Tests\app.config'.

But this is the only failure; the actual Toolkit DLLs all build now with only binary dependencies on SharpDX! So this is real progress!

I need to time out for tonight, but I will work on pushing these changes as pull requests to both the SharpDX and SmartK8 repositories, once I validate that the SharpDX.Toolkit works at all. I will also try running the tests once I get that final missing app.config file from you.

Thanks again!

@RobJellinghaus
Copy link

I've actually pushed my SharpDX changes to https://github.com/RobJellinghaus/SharpDX and my fork of your repository to https://github.com/RobJellinghaus/SharpDX.Toolkit -- feel free to have a look if you are curious :-)

@RobJellinghaus
Copy link

RobJellinghaus commented Oct 3, 2016

I am having an issue with bringing up my app with the above branch of the toolkit and SharpDX. I get an exception thrown during static initialization of SharpDX.Toolkit.Graphics.dll. Specifically, this one:

    internal class Native
    {
        public static unsafe void* Fixed<T>(ref T data)
        {
            throw new NotImplementedException();
        }

The call stack is:

    SharpDX.Toolkit.dll!SharpDX.Native.Fixed<SharpDX.Toolkit.Graphics.EffectShaderType>(ref SharpDX.Toolkit.Graphics.EffectShaderType data) Line 41 C#
    SharpDX.Toolkit.dll!SharpDX.Toolkit.Serialization.BinarySerializer.SerializeEnum<SharpDX.Toolkit.Graphics.EffectShaderType>(ref SharpDX.Toolkit.Graphics.EffectShaderType value) Line 576   C#
>   SharpDX.Toolkit.dll!SharpDX.Toolkit.Graphics.EffectData.Shader.SharpDX.Toolkit.IDataSerializable.Serialize(SharpDX.Toolkit.Serialization.BinarySerializer serializer) Line 125  C#
    SharpDX.Toolkit.dll!SharpDX.Toolkit.Serialization.BinarySerializer.Serialize<SharpDX.Toolkit.Graphics.EffectData.Shader>(ref SharpDX.Toolkit.Graphics.EffectData.Shader value, SharpDX.Toolkit.Serialization.SerializeFlags serializeFlags) Line 541    C#
    SharpDX.Toolkit.dll!SharpDX.Toolkit.Serialization.BinarySerializer.Serialize<SharpDX.Toolkit.Graphics.EffectData.Shader>(ref System.Collections.Generic.List<SharpDX.Toolkit.Graphics.EffectData.Shader> valueList, SharpDX.Toolkit.Serialization.SerializeFlags serializeFlags) Line 903   C#
    SharpDX.Toolkit.dll!SharpDX.Toolkit.Graphics.EffectData.SharpDX.Toolkit.IDataSerializable.Serialize(SharpDX.Toolkit.Serialization.BinarySerializer serializer) Line 153 C#
    SharpDX.Toolkit.dll!SharpDX.Toolkit.Serialization.BinarySerializer.Serialize<SharpDX.Toolkit.Graphics.EffectData>(ref SharpDX.Toolkit.Graphics.EffectData value, SharpDX.Toolkit.Serialization.SerializeFlags serializeFlags) Line 541  C#
    SharpDX.Toolkit.dll!SharpDX.Toolkit.Serialization.BinarySerializer.Load<SharpDX.Toolkit.Graphics.EffectData>() Line 469 C#
    SharpDX.Toolkit.dll!SharpDX.Toolkit.Graphics.EffectData.Load(System.IO.Stream stream) Line 86   C#
    SharpDX.Toolkit.Graphics.dll!SharpDX.Toolkit.Graphics.PrimitiveQuad.PrimitiveQuad() Line 15 C#

So this is the static constructor for PrimitiveQuad defined in SharpDX.Toolkit.Graphics/StockEffects/HLSL/PrimitiveQuad.fx:

    public partial class PrimitiveQuad
    {
        private static readonly SharpDX.Toolkit.Graphics.EffectData effectBytecode = SharpDX.Toolkit.Graphics.EffectData.Load(new byte[] { ... }

The problem is really the final line shown below of the beginning of the EffectData.Shader.Serialize method:

            void IDataSerializable.Serialize(BinarySerializer serializer)
            {
                serializer.Serialize(ref Name, SerializeFlags.Nullable);
                serializer.SerializeEnum(ref Type);
                serializer.SerializeEnum(ref CompilerFlags);  <<<--- boom

CompilerFlags is of type EffectCompilerFlags defined in SharpDX.Toolkit/Graphics/EffectCompilerFlags.cs. So the problem is there is no instantiation of Native.Fixed<EffectCompilerFlags>.

The comment to the SharpDX.Toolkit/Native.cs Native class is:

    /// <summary>
    /// The implementation of this class is filled by InteropBuilder post-building-event.
    /// </summary>
    internal class Native

So it looks like the post-build InteropBuilder step described there is not working properly. Or else there is something else wrong :-)

However, the build output looks like it thinks it is successful at running InteropBuilder on SharpDX.Toolkit:

1>  SharpDX.Toolkit -> C:\Git\SharpDX.Toolkit-SmartK8-RobJellinghaus\Bin\DirectX11-net40\SharpDX.Toolkit.dll
1>  SharpDX interop patch for assembly [C:\Git\SharpDX.Toolkit-SmartK8-RobJellinghaus\Bin\DirectX11-net40\SharpDX.Toolkit.dll]
1>  SharpDX patch done for assembly [C:\Git\SharpDX.Toolkit-SmartK8-RobJellinghaus\Bin\DirectX11-net40\SharpDX.Toolkit.dll]

Yet the resulting SharpDX.Toolkit.dll definitely has no implementations of Native.Fixed<T> methods, at least none that ILSpy can see...

Will keep debugging on this, but if anyone knows the likely issue offhand, by all means please reply! Thanks.

@SmartK8
Copy link
Author

SmartK8 commented Oct 3, 2016

The Native.cs is indeed not implemented and is filled using AfterBuild target with SharpCLI.exe. You might need to modify the SharpCLI in tools and change the lines (also check whether targets file contains SharpCLI.exe Exec.. it should):

563 if (method.DeclaringType.Name == "Interop" || method.DeclaringType.Name == "Native")
654 else if (methodDescription.DeclaringType.Name == "Interop" || methodDescription.DeclaringType.Name == "Native")

It's kinda tough, because I've said I have whole SharpDX build environment set up. AFAIK the Tools folder is not part of this branch.

regards,
Kate

@RobJellinghaus
Copy link

Thank you so much for that tip. It was exactly what I needed. I was able to patch SharpCLI as you suggested in my fork of SharpDX, and then copy it into my fork of the Toolkit. This got around all issues with Native.Fixed. I was then able to fix a bug in my interop code (simply calling the wrong method), and then in very short order I had everything working -- my app lives again on Windows 10!

And as a bonus, I have a Toolkit fork that successfully builds and runs with only binary dependencies on the SharpDX DLLs. I think this is ready to make a pull request for.

However, @xoofx has not chimed in here in a while. Alexandre, are you interested in an update to this PR, in the form of a working Toolkit project that has only binary SharpDX dependencies (not integrated in the same source tree)? There may still be more cleanup you would like, but I could put some time into this. Or, if the toolkit is no longer interesting to you, anyone curious can pull from my forks at http://github.com/RobJellinghaus and get what I am currently running in my app.

Thanks again really very much for your invaluable assistance, SmartK8. My users and I are very grateful!

@xoofx
Copy link
Member

xoofx commented Oct 5, 2016

Hi there, sorry, I'm following this not very closely. Personally (and professionally), I'm no longer using the Toolkit (nor even SharpDX), but I will merge PR for sure, once they are clean and ready.

Quick glance, no need to rename Interop to Native as it implies to patch SharpCli which is unnecessary, just keep Interop name in the Toolkit and this should work just fine.

@SmartK8
Copy link
Author

SmartK8 commented Oct 5, 2016

@RobJellinghaus: I'm glad you made it work.

@xoofx: Agreed. I originally did it to make sure, I was using the right class - to avoid being mixed up with the original Interop. But at this point it can be renamed back, I guess.

I've made it primarily for myself and Rob has his own fork for himself. At this point it's getting scattered. Everyone is making their own version/build. It would be great to have at least an illusion of "official" version. I don't know what Rob fixed. But if he built on top of my fork + some fixes + he renames Native => Interop, it would be better to use his fork (his eventual PR). Or I can incorporate his changes in this PR. I'm fine with either way. Just let me know. I would then build "official" binaries for SharpDX Toolkit 3.0.0.0 for people who just want to use it and not mess with source code. If there even are any people using it left beside us :D

regards,
Kate

@Tiba3195
Copy link

I'm looking at getting a VB.net fork up here soon, there are some parts of the toolkit just like in mainline sharpDX that need fixing too.

I'm not sure how to best go about it..... Just remove any of the none byref functions or do we need the none byref functions? how should I handle this?

Right now I'm just removing the none byref stuff, seems to work ok but I'm not sure what you C# guys want to do.

@dazerdude
Copy link
Contributor

dazerdude commented Dec 6, 2016

If xoofx is moving on, it may be useful to have one of you guys take more of a leadership role in maintaining this project. It'd be nice to merge any of these changes that makes the project compatible with SharpDX 3, release a beta nuget package, and then make and release incremental updates as code is improved and issues are fixed.

@RobJellinghaus seems to have the most recent fork. Would we be good merging that and moving from there? Rob, could you make a pull request?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants