Skip to content

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 16, 2025
Merged

Code Quality: Use ComPtr for all D3D11 pointers #16908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0b0a2b
Use ComPtr for all D3D11 pointers
0x5bfa Mar 8, 2025
21943e5
Revert
0x5bfa Mar 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/Files.App.CsWin32/Windows.Win32.ComPtr.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ public ComPtr(T* ptr)
((IUnknown*)ptr)->AddRef();
}

public void Attach(T* other)
{
if (_ptr is not null)
((IUnknown*)_ptr)->Release();

_ptr = other;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public readonly T* Get()
{
Expand Down
66 changes: 33 additions & 33 deletions src/Files.App/ViewModels/UserControls/Previews/ShellPreviewViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,61 +161,61 @@ private unsafe bool ChildWindowToXaml(nint parent, UIElement presenter)
D3D_DRIVER_TYPE.D3D_DRIVER_TYPE_WARP,
];

ID3D11Device* pD3D11Device = default;
ID3D11DeviceContext* pD3D11DeviceContext = default;

foreach (var driveType in driverTypes)
Windows.Win32.Foundation.HRESULT hr = default;
Guid IID_IDCompositionDevice = typeof(IDCompositionDevice).GUID;
using ComPtr<ID3D11Device> pD3D11Device = default;
using ComPtr<ID3D11DeviceContext> pD3D11DeviceContext = default;
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
Copy link
Member

@hez2010 hez2010 Mar 13, 2025

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.

Copy link
Member Author

@0x5bfa 0x5bfa Mar 13, 2025

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?


// Create the D3D11 device
foreach (var driverType in driverTypes)
{
var hr = PInvoke.D3D11CreateDevice(
null,
driveType,
new(nint.Zero),
hr = PInvoke.D3D11CreateDevice(
null, driverType, new(nint.Zero),
D3D11_CREATE_DEVICE_FLAG.D3D11_CREATE_DEVICE_BGRA_SUPPORT,
null,
0,
7,
&pD3D11Device,
null,
&pD3D11DeviceContext);
null, /* FeatureLevels */ 0, /* SDKVersion */ 7,
pD3D11Device.GetAddressOf(), null,
pD3D11DeviceContext.GetAddressOf());

if (hr.Succeeded)
break;
}

if (pD3D11Device is null)
if (pD3D11Device.IsNull)
return false;

IDXGIDevice* pDXGIDevice = (IDXGIDevice*)pD3D11Device;
if (PInvoke.DCompositionCreateDevice(pDXGIDevice, typeof(IDCompositionDevice).GUID, out var compositionDevicePtr).Failed)
// Create the DComp device
pDXGIDevice.Attach((IDXGIDevice*)pD3D11Device.Get());
hr = PInvoke.DCompositionCreateDevice(
pDXGIDevice.Get(),
&IID_IDCompositionDevice,
(void**)pDCompositionDevice.GetAddressOf());
if (hr.Failed)
return false;

var pDCompositionDevice = (IDCompositionDevice*)compositionDevicePtr;
IDCompositionVisual* pChildVisual = default;
IUnknown* pControlSurface = default;

pDCompositionDevice->CreateVisual(&pChildVisual);
pDCompositionDevice->CreateSurfaceFromHwnd(new(hwnd.DangerousGetHandle()), &pControlSurface);
pChildVisual->SetContent(pControlSurface);
if (pChildVisual is null || pControlSurface is null)
// Create the visual
hr = pDCompositionDevice.Get()->CreateVisual(pChildVisual.GetAddressOf());
hr = pDCompositionDevice.Get()->CreateSurfaceFromHwnd(new(hwnd.DangerousGetHandle()), pControlSurface.GetAddressOf());
hr = pChildVisual.Get()->SetContent(pControlSurface.Get());
if (pChildVisual.IsNull || pControlSurface.IsNull)
return false;

// Get the compositor and set the visual on it
var compositor = ElementCompositionPreview.GetElementVisual(presenter).Compositor;
outputLink = ContentExternalOutputLink.Create(compositor);

var target = outputLink.As<IDCompositionTarget>();
target.SetRoot((nint)pChildVisual);
target.SetRoot((nint)pChildVisual.Get());

outputLink.PlacementVisual.Size = new(0, 0);
outputLink.PlacementVisual.Scale = new(1 / (float)presenter.XamlRoot.RasterizationScale);
ElementCompositionPreview.SetElementChildVisual(presenter, outputLink.PlacementVisual);

pDCompositionDevice->Commit();

pControlSurface->Release();
pDCompositionDevice->Release();
pDXGIDevice->Release();
pD3D11Device->Release();
pD3D11DeviceContext->Release();
// Commit the all pending DComp commands
pDCompositionDevice.Get()->Commit();

var dwAttrib = Convert.ToUInt32(true);

Expand Down
Loading