-
Notifications
You must be signed in to change notification settings - Fork 14
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
egui gfx backend #295
egui gfx backend #295
Conversation
guusw
commented
Jun 30, 2022
- Add egui rendering backend & standalone test
- A bit of restructuring on how bindings are generated.
- Setup bindings to gfx headers using mostly opaque types
- Add a create with bindgen scripts for gfx headers
Codecov Report
@@ Coverage Diff @@
## devel #295 +/- ##
==========================================
- Coverage 76.50% 76.32% -0.19%
==========================================
Files 146 149 +3
Lines 25333 25440 +107
==========================================
+ Hits 19380 19416 +36
- Misses 5953 6024 +71
Continue to review full report at Codecov.
|
@@ -176,7 +181,7 @@ struct ContextMainOutput { | |||
swapchainDesc.format = swapchainFormat; | |||
swapchainDesc.width = newSize.x; | |||
swapchainDesc.height = newSize.y; | |||
swapchainDesc.presentMode = WGPUPresentMode_Fifo; | |||
swapchainDesc.presentMode = WGPUPresentMode_Immediate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't Fifo better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it still switches to that automatically on some platforms that enforce it. but we want to control our own frame-rate so we can target 120 or 240 for example for better UI response times
@@ -431,7 +448,7 @@ void Context::requestDevice() { | |||
deviceDesc.nextInChain = &deviceExtras.chain; | |||
#endif | |||
|
|||
getLogger()->debug("Requesting wgpu device"); | |||
SPDLOG_LOGGER_DEBUG(getLogger(), "Requesting wgpu device"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still wondering why you didn't use existing macros here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to send the graphics logs to it's own logger so I can filter them out / tag them / etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some small things but ok
TARGET_SDKROOT=`xcrun --sdk $1 --show-sdk-path` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this script? I need to run this all the time on Mac? If so annoying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently it's only for iOS.
There's some issues because the build process runs through Xcode and sets the SDKROOT environment variables.
It was similar to this issue TimNN/cargo-lipo#41 (unrelated repo)
Additionally bindgen needs some additional clang parameters to find the correct include files for iOS (similar to https://github.com/gfx-rs/wgpu-native/blob/91e14b8ffda0a5ec1d87fd27b7b9e8e62495720d/build.rs#L99)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you document it? We need some iOS instructions at least.
@@ -27,7 +29,10 @@ static inline std::shared_ptr<spdlog::logger> &getLogger() { | |||
#ifdef WEBGPU_NATIVE | |||
static WGPUBackendType getDefaultWgpuBackendType() { | |||
#if GFX_WINDOWS | |||
return WGPUBackendType_D3D12; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe revert this? Anyway let's keep a closer look I guess. Maybe need to test both in CI too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI actually runs very slowly on D3D12 so I think it's worth keeping this default for now.
The windows tests still run on D3D12 (warp)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea why runs slow? Cos of some mistakes they did like you said last time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, one of those open issues about fixing D3D12. Copying data to buffers is currently very slow
# Conflicts: # src/gfx/egui/test.cpp
# Conflicts: # src/gfx/egui/egui_interop.cpp # src/gfx/egui/test.cpp
# Conflicts: # src/gfx/egui/Cargo.toml # src/gfx/egui/egui_interop.cpp
…ts for gfx bindings # Conflicts: # src/gfx/egui/renderer.cpp # src/gfx/egui/test.cpp
Required to be able to generate bindings to c++ member functions
Prevents having to work around unnecessary rebuils caused by rust-analyzer rust-lang/cargo#7432
Only update texcoord bindings. The texture slots still should be defined on features only