-
Notifications
You must be signed in to change notification settings - Fork 494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sokol_app UWP implementation #351
Conversation
Wow that's awesome! It'll be a little while until I'll be able to look into and merge the PR, but I definitely want to get this in. |
Ok, I'll start looking into this PR now. Is the PR ready for merging? (apart from the conflicts which I'll take care of). I've been quickly skimming over the code and found nothing to complain about, but I'll take some time to thoroughly understand the code (and I'll also add a uwp sample subdirectory to sokol-samples). Btw, what's the code in this file for? https://github.com/Aftnet/sokol_app_uwp/blob/master/src/sokol/sokol_uwp_impl.cpp It looks like it's mostly a duplicate of what's in the PR? |
Great to hear. How did you get to that file? It should not be part of the PR, or even in the code base at all at this point. I started working on the UWP implementation in a separate file to make things easier for myself, but moved everything to the header in the end. Also, don't remember if I mentioned this before or in the file itself, but to make things work with UWP one needs to have Here's what I do myself: implement.c
implement.cpp
Having one c/cpp file to implement each sokol header works too of course. |
Yikes ok. Did you try whether calling the D3D11 C interface would work from the UWP specific code? Or is it already some UWP specific header which includes d3d11.h and requires the D3D11 C++ interface? I'd really like to find a solution for this before the merge. Another solution that should definitely work is to call the D3D11 C++ API if the sokol_gfx.h implementation was included in a C++ source. If the sokol_app.h UWP code path can't use the D3D11 C interface, then that's something I'll work on first before the merge. |
I was looking around there because you posted the link to the project in the first comment ;) I'll also try to get UWP support into fips first and create a simple uwp_entry.cc (similar to d3d11_entry.c) in the sokol-samples repository. |
Personally I'd rather go with the second option, mainly because using the C++ interface to D3D11 allows for using smart COM pointers and so prevents memory leaks. The UWP code I used as base comes from Microsoft itself (it's the Visual Studio UWP/D3D app template, which I ported to pure C++ from the original C++/CX) and relies on smart pointers to work - and I would not want to mess with the design. Edit: actually I am not sure it's even possible to call the D3D11 C interface from UWP. I gave it a brief try, without success, and then realized what I said about smart pointers and decided it was not worth pursuing. |
Ok, then I'll create an issue for myself to transparently use the D3D11 C++ API in sokol_gfx.h if the implementation is compiled as C++. |
Awesome, thanks :) |
I think it's great to have UWP support in Sokol; nice work! I have a question about the use of shaders however. In sample code provided the shaders are compiled from text source within the exceutable however I understood UWP on XBox One to forbid this and you had to precompile them? This may not be a problem when using the sokol-shdc cross-compiler as that generates C header files which would work fine in UWP I assume? But I presume you've tested this on XBox one? |
There are several options:
|
FYI: sokol_gfx.h and sokol_app.h now use the D3D11 and DXGI C++ APIs when the implementation is compiled as C++. I will now start looking into this PR. |
Ok I have manually merged the PR into this branch (because of the merge conflicts, it wasn't anything "serious" though) and will continue testing there before merging into master: |
Once again, thank you very much I have updated my own project to the latest Two things I have noticed:
The above are not deal breakers (Apple stuff already needs to be compiled as ObjC), was just wondering if you were aware. |
BTW do you want me to test |
This intended, it's also required for other platforms (this behaviour may change later though, but this would for instance require to move the "MiniGL loader" from sokol_app.h into sokol_gfx.h... long term that may be better, but right now the sokol_app.h implmentation should be included before the sokol_gfx.h implementation when both are used together).
This is surprising, can you maybe open a separate issue with the errors you are seeing?
Not yet, I only resolved the merge collissions and didn't do much testing, I will first try to get everything else in place to compile and test UWP apps on my side. This may require some more changes in fips and the sokol-samples repository), when this is done some tests on your side would be nice before the merge to master, but I'll let you know when its time here in this thread :) |
...maybe a reason could be that you only replaced sokol_gfx.h but use your own sokol_app.h, not sure if that's it, just something that popped into my mind right now. |
Yeah, I also thought that may be the case after I wrote that. Everything seems good then :) |
@albertofustinoni another question: what Visual Studio version are you using? I'm getting some mysterious compilation errors and I'm starting to suspect that VS2017 doesn't have complete C++17 support (I'll try the VS2019 Community Edition next). |
Hmm, it's not VS2019 vs VS2017, I'm getting those errors (in case you've seen those too and know what's the problem, otherwise don't worry, I'll figure it out somehow):
|
You need to be on VS2019 for C++/WinRT support (in 2017 it was experimental at best), also due to improved C++ conformance of 2019. Beyond that, as indicated here, you will need a few things
I have the extension installed on my systems because it adds debugger support (and no NuGet package in my build, I build with CMake and have not added it to the CMakefile) and I build reliably. |
Hmm no luck so far, the core error which is repeated a few times in other places is this:
The other interface where this happens is |
The official sample here compiles fine, but there seems to be some sort of code generation involved: https://github.com/microsoft/Windows-universal-samples/tree/master/Samples/Simple3DGameDX/cppwinrt After compilation there's a "Generated Files" directory which contains all the winrt headers in some sort of preprocessed form. If I remove the |
...right know it looks like the problem is that I don't have the cppwinrt NuGet package which silently overrides the Windows SDK WinRT headers. At least that's the difference between my project and the SimpleDXGame project. The SimpleDXGame project has the NuGet dependency defined in the Visual Studio solution, but I really don't know if I want to go there. I'll see if cmake has a simple solution (it seems to have with VS_PACKAGE_REFERENCES...). Of course now I see that you wrote all of that:
heh :) I added the C++/WinRT extension, but that didn't help. Somehow I skipped the NuGet part when reading. |
Ok it compiles when adding the CppWinRT NuGet package in Visual Studio to the sokol build target. But I really need to find a way to do this via cmake (this VS_PACKAGE_REFERENCES didn't work out of the box, but I'm probably using it wrong...). |
Ok, I got it basically working, just a few things:
I'll spend a few days getting familiar with the code, e.g. I wonder if having those separate DeviceResources and Renderer classes is really needed or useful... (I guess that's how the UWP example programs are structured, but I also don't want to change the code too much unless it really makes sense). One question: currently there's no "command line argument handling", do you know if UWP apps can be passed command line args? If yes we should support this (e.g. my tiny emulators pretty much depend on command line args). |
Oh interesting: I have worked on the latest Win10 SDK (10.0.19041.0) since the beginning, which is why I never noticed anything. Never tried using imgui, although at some point I may need to look into it (will need some in app UI eventually). Does it work for touch or is it only for mouse/keyboard? As for command line handling, I think it is supported, bu I am not sure: UWP apps are meant to be launched from start menu icons for the most part, but I think MS added support for command line UWP apps too at some point. |
A little update: one thing that was messed up in the mouse button code was that left and middle mouse button was mixed up: Lines 5282 to 5289 in 9929134
But now I'm stuck with detecting which mouse button was actually released, because I'm only getting this generic PointerRelease event, but I'm not seeing a way how to detect which mouse button was released. I'll continue with this tomorrow... |
...ok fixing the mouse button input was actually quite the rabbithole, because UWP also doesn't send Pressed/Released events when multiple buttons are pressed simultaneously, even SDL seems to get this wrong. I came up with the following solution, which seems to work. I need to test more input edge cases though: Lines 5715 to 5758 in 2b717d8
|
Wow, I am sorry: I did the mouse implementation quickly because I don’t really need it but wanted to get as close to feature parity as I could with the other platforms and mistakes slipped in. Never thought it would turn into this much of a rabbit hole. |
No need to be sorry :) |
Hmm more UWP strangeness... The window.Closed event handler never seems to be called (this also means the loop in App::Run is never finished, the application simply terminates. Instead when clicking the window-close-button, App::OnSuspending() is called. I wonder now where to best put the _sapp_call_cleanup() call. OnSuspending() seems to be the wrong place, because that implies that OnResuming() could be called later. I'm also not seeing other event handlers like "OnTerminated()"... PS: probably like this: https://docs.microsoft.com/en-us/windows/uwp/launch-resume/app-lifecycle#app-close PPS: Never mind, that doesn't work, it's only possible to detect on the next start whether the application was closed by the operating system or by the user. What a mess UWP is... I will simply remove the window.Closed handler as it isn't called anyway, which simply means the sokol-app cleanup callback will never be called, this is similar to how it is when runnin on the web platform, so no big deal. |
Ok, nearly done now, I think there's only one topic remaining: DPI-handling. On my HighDPI Windows laptop the UWP backend behaves entirely different than the Win32 backend (the UWP backend always uses HighDPI rendering, no matter what the sokol-app high_dpi flag says, because the UWP backend uses some random threshold values (192 dpi, 1920x1080 pixels) to decide whether rendering should happen in high-dpi or not. Apart from always using HighDPI rendering on my laptop, this also throws off the mouse coordinates, they seem to be off in all samples when running on this laptop. I'll need to figure out how to rewrite the HighDPI handling in the UWP backend so that it behaves identical to the other platforms, so it'll be a couple more days until merging... |
PS: the new mouse-lock feature also requires work, but I'll simply mark this is "TODO" in the feature matrix, not a showstopper for the merge. |
Note to self:
|
I just noticed that MSAA support is missing, I'll start working on this now (good thing is that I can mostl likely use the same approach later for the Win32/D3D11 backend, because this currently prints a deprecated warning, DXGI MSAA support in the swap chain must be implemented similar to WebGPU, with a separate MSAA render target and an explicit resolve into the framebuffer surface). PS: or maybe I'll do the merge first, I'm already too long on this branch... |
@albertofustinoni I think you can give the Aftnet-sapp_uwp branch a whirl now, this is the version I'd like to merge to master (only thing missing is some notes in the sokol_app.h documentation header, I'll finish that today or tomorrow). |
@floooh Just gave your branch a spin and linker complains about wWinMain being undefined. Then I noticed your comment above about Unicode and I should say Windows applications (not just UWP) need to be compiled with Sadly Windows just does not do UTF8 at all: when you have API calls that have a Apps that work with UTF8 strings need to convert to wide chars when calling into the API and back. See here for reference. I would strongly recommend you have |
This isn't really a problem at least when using Win32 calls, you can compile Windows applications without the UNICODE define but still use the wide-string API calls by explicitly calling *W() functions. The Windows code page settings are also irrelevant, those settings only change the behaviour of the *A() (ASCII) Windows API functions, and those are never called. (...it seems like C++/WinRT has its own string type based on UTF-16, a bit of a shame that they didn't make the switch to UTF-8, but this doesn't really matter for the sokol headers: https://kennykerr.ca/2018/02/28/cppwinrt-strings/) The usual strategy is to treat all application-internal strings as UTF-8 encoded, and only covert from and to UTF-16 (in Windows lingo "UNICODE" although that's incorrect) when calling Windows API functions. See here for instance in the sokol_app.h Win32 code: Lines 5332 to 5344 in 342821c
This is the "UNICODE" Version of CreateWindowEx() which accepts UTF-16 encoded "wide-strings", the window title is passed as UTF-8 encoded string to sokol-app, and converted to UTF-16 via MultiByteToWideChar: Lines 4790 to 4803 in 342821c
I've been using that approach for the last 15 years or so and it works great, it's also what's recommended in the utf8everywhere manifesto: I'll think about checking if UNICODE is defined and then use wWinMain() instead of WinMain(), but it might be confusing for users because all the Sokol-APIs will still require strings to be passed in as UTF-8 encoded, not UTF-16, and that's where I won't budge ;) |
I think we are actually saying the same thing: app code, including sokol, should be using UTF8 and only translate to UTF16 when dealing with Windows APIs. I am definitely not arguing for UTF16 usage anywhere else. Allowing compilation with UNICODE defined (converting |
@albertofustinoni do you see a Character Set configuration option in your Visual Studio project settings? I'm a bit confused because I don't see that in my C++/WinRT project (only in Win32 projects). I got a wWinMain() linker error in the original code and had to switch to WinMain(), so I guess there must be an option somewhere to switch UNICODE on and off, I'm just not finding the option anywhere. I want to test a hybrid entry point (use wWinMain when UNICODE is defined, and WinMain otherwise), but on one hand I don't know why my project (created with cmake) defaults to UNICODE switched off, and on the other hand I'm confused because I'm not seeing a related option in the Visual Studio project settings dialog (like I'm seeing in a Win32 project). |
Ah nvm, I think I figured it out. Instead of a separate compiler option this simply seems to be done now through standard preprocessor defines (e.g. UNICODE and _UNICODE are part of the %(PreprocessorDefinitions) "config variable". |
hmmm ..strangely enough, both
This also sort of kills my plan to use wWinMain() when UNICODE is defined and WinMain() otherwise. I need to figure out why my build config expects WinMain() first... |
Another weirdness is that the CoreApplication C++/WinRT example accepts both wWinMain() or WinMain() without any linker errors. So far I haven't found any compiler/linker option differences which would explain the different behaviour. It seems to be about the Runtime Library that's being linked, but I can't see a difference there, nor in the linker's "subsystem". |
Hello Unfortunately I’m gonna be away from home and PC for the next three days, so I won’t be able to help much :( In my experience too WinRT projects do not have a codepage setting, and wWinMain should be enabled by defining |
Ok, defining UNICODE doesn't seem to select between WinMain or wWinMain (the Windows SDK headers always define both entry points). The linker error seems to be related to the runtime libraries that are linked. I'll dig a bit deeper today and try to find out what's different to the C++/WinRT samples that come with the Visual Studio extension (interestingly, those accept both WinMain or wWinMain). |
Ok, I think I found the solution, but it's very non-obvious. Basically if wWinMain() is defined in a static library, the linker won't recognize it, and this magic line is needed in code that links against this library: Line 1082 in 88fb6cd
(which I'm doing now in the declaration part when compiling for C++/WinRT, so that this line is "injected" into each code module which uses sokol_app.h). If this line exists, an unresolved-error happens if there is no wWinMain(). Otherwise there seems to be some sort of autodetection happening when either WinMain() or wWinMain() is defined in a "top-level" object-file which is fed directly into the linker (not indirectly via a static link library), because when I added either a WinMain() or wWinMain() directly to my samples (or included the sokol_app.h implementation directly into the sample source code), both WinMain() or wWinMain() worked (e.g. WinMain also worked when UNICODE is defined, which is the case in my WinRT/C++ projects, since that's automatically defined by the Visual Studio build system). When you have a minute, please check whether this works now on your side (sokol_app.h now selected wWinMain() if UNICODE is define, otherwise WinMain(): Lines 6649 to 6653 in 88fb6cd
|
Btw, question @albertofustinoni: I've tried to setup my Xbox One for testing this stuff but it seem this needs a paid "Partner Center" account now. As far as I remember this "just worked" on the 360 without a separate account. Are there no free options anymore? |
Just had a chance to test your changes and now the code in Massive thanks for everything so far. As for the Partner Center account: looks like they made it paid for now :( |
Alright, thanks for testing! I'll merge the branch back to master within the next few days then :) I think I'll bite the bullet and pay for a private account, AFAIK it's just a one-time payment. Now that it's possible I also want to see the code run on the Xbox One. Btw, Apple now offers dev accounts that are free and allow deploying and debugging on your device, the $99 tax is only needed if you want to publish the application ;) |
Awesome, thanks again I thought the free Apple dev account forces apps on devices to expire after one week. Has this changed recently or is there a workaround? BTW, would you be interested in a gamepad API? I have made my own following sokol’s code style and it works on Windows (desktop/UWP via XINPUT), Mac/iOS and Android. |
Yeah there are definitely limitations, you can also only have up to 8 different apps on the device at the same time.
That's definitely something I'd want to look at because my rough plan for the next sokol headers looks like this:
|
Ok merged! |
Implemented
sokol_app
for UWP.Added supported features in table: basically close to parity to desktop Win32 (clipboard and cursor capture/replacement only missing). Hopefully this can do for a start :)
Tested and working on my PC and Xbox One via project here