Direct display feature for the Vulkan backend #3759
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…a display surface, making possible to render directly to the display without windowing system. Currently this is achieved using Vulkan extensions, so this is a Vulkan only feature.
Removed "unsafe" from functions since there is no chance of undefined behaviour Added missing `image_extent` parameter to `create_display_surface`, even if from preliminary tests, seems to have no effects. The surface seems to use only DisplayMode to select the surface size.
The new api should be more simply while keeping all the features offered by vulkan. It is also less vulkan-specific, maybe allowing in the future to other rendering api to use the direct display path.
…ome checks on the create_display_plane_surface to deny undefined behaviours.
…_KHR_display extension. Changed the error structure returned by enumerate_available_displays to include the possibility that the system does not support the feature. Since the enumerate_available_displays is the "entry point" to take advantage of the feature (and others related functions cannot be called before that), the checking has been added only to that function.
…splay, and it require ust V1.0 . So checks about vulkan version on enumerate_available_displays has been removed. Also removed the vulkan version check on the instance creation when VK_KHR_display is added to the extension list.
…roubleshoot a common problem.
Merged a function from src/backend/vulkan/src/display.rs into the only function that use it and removed the file.
Moved all the new functions except `create_display_plane_surface` to `PhysicalDevice`.
Implemented From<(image::Size,image::Size)> for Extent2D. Implemented From<(image::TexelCoordinate,image::TexelCoordinate)> for Offset2D. Replaced all the tuple instances in hal::display with the appropriate Offset2D and Extent2D.
…initial implementation.
Replaced (u32,u32) with Extend2D on the "create_display_plane_surface" function for all the backends.
Added to direct display related function a cfg to target linux only since it is a linux only feature. Direct dispay related functions for non-linux backend (like dx and metal backends) has been commented since they will never be compiled. I cannot test all the platform, that's why they are only commented. If the CI tests will pass, i will delete the commented functions.
Removed "enumerate_builtin_display_modes" and builtin modes will be returned in the "enumerate_available_displays" function.
…isplay_control` vulkan extension.
…unctions accorting to the new api
… is no external dependency to be added.
…d by vulkan validation layers that i need to investigate.
Added From<Extent2D> for (image::Size, image::Size) implementation so that Extent2D can be converted into tuple. Fixed a problem with the resolution of the quad example for the direct display path: it was using the first display mode available instead of using the resolution exposed by the DIMS variable. Now it look for the DIMS resolution into the built-in modes and will be created if not found. If the creation fails, the first available built-in mode will be used and the renderer resolution will be setted accordingly.
On the quad example, setted the display plane size as the resolution of the used display mode, so that if it is not possible to use DIMS, it will be anyway a valid resolution.
…ead of instance extension.
Changed direct display function to unsafe
Removed leftover code. Removed custom partial eq implementation that is no more needed.
Changed the return error of `create_display_plane` to OutOfMemory because the only other possible error cannot be triggered due to the shape of the api. Fixed non camel case enums on the quad example.
|
@Uniformbuffer3 what is the state of this PR? |
|
Generally is ready, display and surface related functions are working correctly, but i have a problem with a function that comes from |
|
I have tested the Still i have tested the other parts of the api and they work. I'm preparing a quick test program if someone want to test these features with a different system. |
kvark
reviewed
May 31, 2021
kvark
requested changes
May 31, 2021
| @@ -750,6 +752,20 @@ impl PhysicalDevice { | |||
| None | |||
| }; | |||
|
|
|||
| let display_control = if enabled_extensions.contains(&vk::ExtDisplayControlFn::name()) { | |||
There was a problem hiding this comment.
we need to file an issue to Ash to expose this better, similarly to how DrawIndirectCount is exposed
Removed `.buildconfig` that has been added by mistake.
On `DisplayPlane` reduced the members by merging "min max" fields using `std::ops::Range`. Removed leftover code. Changed signature of `enumerate_displays` and `enumerate_compatible_planes` so that they return directly `Vec<display::Display<B>>`. Removed from `create_display_mode` the UnsupportedFeature error since there is no chance it will be returned.
Replaced a check against `std::ptr::null()` with `is_null()`
… because it is not a promoted extension, so there is no need to use `ExtensionFn`, but just `Option<vk::ExtDisplayControlFn>`.
…can be builded from/to vulkan bits. Due to this, the related functions in conv has been removed, the same result can be achieved with just a line now.
…ed: if true it is requested, not otherwise.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi, i have implemented the ability to draw directly on screen for the Vulkan backend using the
VK_KHR_displayextension.Also the
VK_EXT_display_controlextension has been implemented, so that it is possible to get notifications about disconnecting monitors and it gives the ability to control the power state of the monitor (on, off, suspended).The "quad" example has been modified so that, if
DIRECT_DISPLAYenv variable is setted, it will try to draw on the first monitor returned from the system. Since "quad" is an example, i have clearly separated the two paths with a big if statement, so that users do not get confused about this new feature.The display power control has been tested with a separate test crate, but i can of course provide the code if you want it.
Also to be pointed out, Vulkan will try to enumerate all possible monitors that are not controlled by other systems. If all the monitors are already controlled by some programs (like an already running compositor/desktop environment), the enumerate function will return an empty list.
This is mainly a Linux feature, but i have discovered that it is possible to get the control of a monitor even on Windows if that monitor is not controlled by the Windows compositor (but unfortunately i can make tests only on Linux).
If you are interested in such feature, i would like to know your opinion about this and get your feedback so that i can improve it .
Thanks for the dedicated time,
Have a good day
Tests:
To test the direct display surface creation, a special path has been implemented into the quad example. To use it the
DIRECT_DISPLAYenv variable need to be present before launching the example. Please keep in mind that if the example (using this mode) is launched on an already running desktop environment, there is an high chance that it will fail beause there is no available monitor the application can take the control (because already controlled by the running compositor). To be sure it works launch it directly on a tty. The example will run for 5 seconds and terminate. On Linux and based on the driver implementation, the tty could not return automatically in text mode. Switching from and back to the tty will bring back the text mode.It is also possible to use this feature on Windows by telling the Windows compositor to do not use one of the monitors.
I'm not too familiar with Windows, but i have found a git repository that explain how to this: https://github.com/nvpro-samples/gl_render_vk_ddisplay
That monitor will be then available for the display enumeration of this feature and the Gfx logo should appear it (still, i don't have a Windows machine, so this has not been tested on Windows).
Also functions from
VK_EXT_display_controlhas been implemented and tested using this quick and dirty test program: https://github.com/Uniformbuffer3/gfx_direct_display_test .On the main function are present the functions that will test these features (remove the comment syntax from the desired function).
Tests result on my system (Intel Broadwell + Nvidia Gtx 850M Optimus system):
I suppose some of these tests fails due to the fact that mine is an Optimus system; they are famous to have weird/unresolved bugs related to display syncronization and comunication due to the way Optimus systems works. If someone want to test this on a non-Optimus system, it would be very appreciated.