-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Code Quality: Use ComPtr for all D3D11 pointers #16908
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
Code Quality: Use ComPtr for all D3D11 pointers #16908
Conversation
|
@hez2010 What and where exactly was the issue that caused the crash in Preview Pane? Looks like this PR has a crash when disposing CEOL again. Perhaps should I dispose these com objects manually? |
Some resources shouldn't be released eagerly when the visual content is still visible. |
|
I disposed an object (pChildVisual) that shouldn't be. Thanks, it looks to be working now. |
aa719f3 to
b0b0a2b
Compare
|
@yaira2 Ready. I'm gonna remove Vanara from this area later when i take a break for Omnibar. |
| using ComPtr<IDXGIDevice> pDXGIDevice = default; | ||
| using ComPtr<IDCompositionDevice> pDCompositionDevice = default; | ||
| using ComPtr<IUnknown> pControlSurface = default; | ||
| ComPtr<IDCompositionVisual> pChildVisual = default; // Don't dispose this one, it's used by the compositor |
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.
It seems that we never release it and would cause memory leaking. I think we should do something around this to free it eventually when user switched or closed the preview pane.
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.
this was what it was though. however, we can make this to a class-level field and dispose when disposing CEOL?
hez2010
left a comment
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.
LGTM!
|
@hez2010 I just tried to dispose pChildVisual but it seems to be already disposed by CEOL. What should we do, shoul we leave as was? FWIW, swapping the line of disposing CEOL and that of disposing pChildVisual causes the same crash in CEOL.Dispose() instead. |



Resolved / Related Issues
General improvements for code that uses raw unmanaged pointers.
Steps used to test these changes
CC @hez2010