add sdl2 and sdl3 WSI backends#133
Conversation
|
it also runs on x86_64, configure.sh just needed some touch-ups, i'll push them in a separate branch and make a PR for that in a bit 
|
3Shain
left a comment
3Shain
left a comment
There was a problem hiding this comment.
I can't comment on the WSI implementations (other than LGTM). So I'll merge it after feedbacks get resolved.
| #if defined(DXMT_WSI_SDL2) || defined(DXMT_WSI_SDL3) | ||
| native_view_ = wsi::createMetalViewFromHWND((intptr_t)hWnd, pDevice->GetMTLDevice(), layer_weak_); | ||
|
|
||
| if (!native_view_) { | ||
| ERR("Failed to create metal view."); | ||
| abort(); | ||
| } | ||
| #else | ||
| native_view_ = WMT::CreateMetalViewFromHWND((intptr_t)hWnd, pDevice->GetMTLDevice(), layer_weak_); | ||
|
|
||
| if (!native_view_) { | ||
| ERR("Failed to create metal view, it seems like your Wine has no exported symbols needed by DXMT."); | ||
| abort(); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
They look almost the same, macro is unnecessary and log can be moved into wsi implementation.
There was a problem hiding this comment.
And please reorganize the commits.
- It's confusing to see SDL3 when your commit description is about SDL2.
- Changes in scope of d3d11 deserve a dedicated commit.
| #if !defined(DXMT_WSI_SDL2) && !defined(DXMT_WSI_SDL3) | ||
|
|
There was a problem hiding this comment.
I see a lot of checks on defined(DXMT_WSI_SDL2), etc...
But they are unnecessary given you already selectively add source files based on new added options.
I agree with you that it doesn't make sense to support multiple WSIs, so you don't need to define DXMT_WSI_* macros either. (although DXMT_NATIVE is still needed... well is it? irrelevant to this PR though.)
There was a problem hiding this comment.
Forgot to say this file and wsi_window_sdl3.hpp are not used anywhere?
2 participants
slightly touched up sdl2 from the old PR and added sdl3 as well. i didn't test sdl3 as thorougly yet, but the ImGui example runs fine.
initially i wanted to mimic dxvk's env var system for picking WSI backend at runtime, but it would be a bit of a pain to maintain and realistically i don't think anyone would ever want more than one WSI backend.
EDIT: if i have some time i'll try to get this to work on amd64 macs as well, currently
./configure.sh --nativefails (due to trying to build for aarch64 for some reason...)sample code (let me know if sample is broken, i tested it outside the repo before updating it): https://github.com/sse2/dxmt-native-sample/tree/main
