Use DComp surface handle for Swap Chain management #10023
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
zadjii-msft
added
Area-Rendering
80 tasks
miniksa
reviewed
May 3, 2021
DHowett
requested changes
May 4, 2021
ghost
added
Needs-Author-Feedback
3 tasks
DHowett
reviewed
May 11, 2021
| wil::unique_hmodule hDComp{ LoadLibraryEx(L"Dcomp.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32) }; | ||
| RETURN_LAST_ERROR_IF(hDComp.get() == nullptr); | ||
|
|
||
| auto fn = GetProcAddressByFunctionDeclaration(hDComp.get(), DCompositionCreateSurfaceHandle); |
DHowett
approved these changes
May 11, 2021
|
DO NOT AUTOMERGE (kills the stacked PRs) |
ghost
requested review from
lhecker
reviewed
May 12, 2021
| // actual failure from the API itself. | ||
| [[nodiscard]] HRESULT DxEngine::_CreateSurfaceHandle() noexcept | ||
| { | ||
| wil::unique_hmodule hDComp{ LoadLibraryEx(L"Dcomp.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32) }; |
There was a problem hiding this comment.
The library is technically called dcomp.dll. I know that Microsoft's prior file systems are all case insensitive unfortunately, but... you know... It might be a good idea to fix the name.
Assuming that this function is only called from the main thread you can also write:
(Removed, as making code more complex really isn't that useful...)
| @@ -79,6 +79,7 @@ DxEngine::DxEngine() : | |||
| _backgroundColor{ 0 }, | |||
| _selectionBackground{}, | |||
| _haveDeviceResources{ false }, | |||
| _swapChainHandle{ INVALID_HANDLE_VALUE }, | |||
There was a problem hiding this comment.
wil::unique_* types have default constructors and you don't need to initialize them here.
lhecker
approved these changes
May 12, 2021
|
Idle thought: should we use the APISet name for dcomp.dll? It might be more "future proof" or whatever. |
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.
Summary of the Pull Request
This PR changes the DxEngine to create a swapchain HANDLE, then have the TermControl attach that handle to the SwapChainPanel, rather than returning the swapchain via a
IDXGISwapChain1.I didn't write this code originally, @miniksa helped me out. The original commit was so succinct that I didn't think there was anything else to add or take away.
I'm going to need this for tear-out (#1256), so that I can have the content process create swap chain handles, then duplicate those handles out to the window process that will end up embedding the content.
References
DCompositionCreateSurfaceHandleCreateSwapChainForCompositionSurfaceHandleCreateSwapChainForCompositionPR Checklist
Detailed Description of the Pull Request / Additional comments
This reverts commit c113b65.
That commit reverted 30b8335
Validation Steps Performed
ForHwndpath, so this will still work, but I don't know if this will suddenly fail to launch on Win7 or something. Tagging in @miniksa.