Skip to content
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

Add GLFW WSI for native builds #3111

Closed
wants to merge 8 commits into from
Closed

Conversation

Beyley
Copy link
Contributor

@Beyley Beyley commented Dec 3, 2022

This is a WIP, and is based on my PR from the original dxvk-native repository, posting here early to get feedback, as im not familiar with C++ nor the Meson build system

Required a hack to a vulkan loader file, as without it compilation failed for missing symbols and i could not figure out why

@Beyley Beyley marked this pull request as draft December 3, 2022 03:15
@K0bin
Copy link
Collaborator

K0bin commented Dec 5, 2022

Whats the advantage of using GLFW over SDL2 like we currently do?

@doitsujin
Copy link
Owner

doitsujin commented Dec 5, 2022

I'm not opposed to adding GLFW support; I guess the main question is whether this is actually going to be used or if it's just a proof-of-concept.

Required a hack to a vulkan loader file, as without it compilation failed for missing symbols and i could not figure out why

Make sure that any file that includes GLFW headers includes Vulkan headers first.

Also, as general feedback (if we do intend to merge this), we generally use two spaces for indentation, not four.

@Beyley
Copy link
Contributor Author

Beyley commented Dec 5, 2022

Whats the advantage of using GLFW over SDL2 like we currently do?

There isn't a direct advantage, but it allows existing GLFW DX11 apps to be ported easily

I guess the main question is whether this is actually going to be used or if it's just a proof-of-concept.

I'm specifically implementing this to be able to add DXVK-native binaries for Silk.NET, so that any games using our D3D11 bindings can just import a NuGet package, and we'll just figure out the rest in the background for them

And regarding the formatting, i did notice that, ill fix that next chance i get

@Beyley
Copy link
Contributor Author

Beyley commented Dec 5, 2022

I fixed the formatting and the vulkan_loader issue

@Beyley
Copy link
Contributor Author

Beyley commented Dec 5, 2022

image
Heres my game engine running on linux using D3D11 under GLFW, to show that it fully works in its current state (at least with my game engine's usage of D3D11)

I think there needs to be an easier way to switch the DXVK build from GLFW to SDL, should i add something to meson_options.txt like it was in the original dxvk-native repo?

@Beyley Beyley marked this pull request as ready for review December 11, 2022 20:02
@Beyley
Copy link
Contributor Author

Beyley commented Dec 11, 2022

After a week straight of working on my game through the GLFW WSI, i think its ready for proper review (i still need to add a better selection mechanism for WSI though)

src/dxvk/platform/dxvk_glfw_exts.cpp Outdated Show resolved Hide resolved
src/wsi/glfw/wsi_monitor_glfw.cpp Outdated Show resolved Hide resolved
src/wsi/glfw/wsi_monitor_glfw.cpp Outdated Show resolved Hide resolved
@Beyley Beyley changed the title Draft: Add GLFW WSI for native builds Add GLFW WSI for native builds Dec 14, 2022
@doitsujin
Copy link
Owner

Indentation is still all over the place (some files use tabs for some reason?), other than that it should be good to go.

@Beyley
Copy link
Contributor Author

Beyley commented Dec 16, 2022

Indentation is still all over the place (some files use tabs for some reason?), other than that it should be good to go.

indentation for the SDL one is also a bit all over the place, sometimes with 4 spaces sometimes with 2 spaces (i tried to match it as close as possible), maybe it'd be worth adding a clang format file to the repo? then i can just rebase and run a format over all my files (i think verifying the formatting is correct, is possible to be done as a CI job to)

@pchome
Copy link
Contributor

pchome commented Dec 16, 2022

@Beyley

maybe it'd be worth adding a clang format file to the repo? then i can just rebase and run a format over all my files (i think verifying the formatting is correct, is possible to be done as a CI job to)

Learn about "vertical text selection" thing, for me it's Ctrl+Atl+"select with mouse" in terminal, and Sift+Alt+"set cursor to start position and select with mouse from this position" in VSCode. YMMV

So in IDE select everything from start to function names, replace with two spaces, select everything inside function block, replace ...

FYI

@Beyley
Copy link
Contributor Author

Beyley commented Dec 16, 2022

@Beyley

maybe it'd be worth adding a clang format file to the repo? then i can just rebase and run a format over all my files (i think verifying the formatting is correct, is possible to be done as a CI job to)

Learn about "vertical text selection" thing, for me it's Ctrl+Atl+"select with mouse" in terminal, and Sift+Alt+"set cursor to start position and select with mouse from this position" in VSCode. YMMV

So in IDE select everything from start to function names, replace with two spaces, select everything inside function block, replace ...

FYI

for changing indentation i use the button to change indentation in vscode, idk why i'd use multiple cursors here

@Beyley
Copy link
Contributor Author

Beyley commented Jan 2, 2023

i think my laptop's vscode was configured wrong, on my desktop i can see now that all the SDL2 files are 2 spaces (maybe vscode was autoconverting to tabs as thats what i use in my personal projects?)

Copy link
Collaborator

@misyltoad misyltoad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nits, then we can merge this.

src/wsi/glfw/wsi_monitor_glfw.cpp Show resolved Hide resolved
src/wsi/glfw/wsi_monitor_glfw.cpp Outdated Show resolved Hide resolved
@doitsujin
Copy link
Owner

Merged manually.

Apologies that this took so long but christmas holidays kind of got in the way too.

@doitsujin doitsujin closed this Jan 8, 2023
@Beyley Beyley deleted the glfw-wsi branch January 8, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants