Skip to content

Commit da88530

Browse files
0x5bfa0x5bfa
authored
Code Quality: Improved SetAs actions (#16480)
1 parent 5b5541d commit da88530

File tree

8 files changed

+74
-72
lines changed

8 files changed

+74
-72
lines changed

‎src/Files.App.CsWin32/Windows.Win32.ComPtr.cs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Runtime.CompilerServices;
66
using Windows.Win32;
7+
using Windows.Win32.Foundation;
78
using Windows.Win32.System.Com;
89

910
namespace Windows.Win32
@@ -39,11 +40,19 @@ public ComPtr(T* ptr)
3940
}
4041

4142
[MethodImpl(MethodImplOptions.AggressiveInlining)]
42-
public readonly ComPtr<U> As<U>(Guid riid) where U : unmanaged
43+
public readonly ComPtr<U> As<U>() where U : unmanaged
4344
{
44-
ComPtr<U> pNewPtr = default;
45-
((IUnknown*)_ptr)->QueryInterface(&riid, (void**)pNewPtr.GetAddressOf());
46-
return pNewPtr;
45+
ComPtr<U> ptr = default;
46+
Guid iid = typeof(U).GUID;
47+
((IUnknown*)_ptr)->QueryInterface(&iid, (void**)ptr.GetAddressOf());
48+
return ptr;
49+
}
50+
51+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
52+
public readonly HRESULT CoCreateInstance<U>(CLSCTX dwClsContext = CLSCTX.CLSCTX_LOCAL_SERVER)
53+
{
54+
Guid iid = typeof(T).GUID, clsid = typeof(U).GUID;
55+
return PInvoke.CoCreateInstance(&clsid, null, dwClsContext, &iid, (void**)this.GetAddressOf());
4756
}
4857

4958
[MethodImpl(MethodImplOptions.AggressiveInlining)]

‎src/Files.App/Actions/Content/Background/BaseSetAsAction.cs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ namespace Files.App.Actions
88
{
99
internal abstract class BaseSetAsAction : ObservableObject, IAction
1010
{
11-
protected readonly IContentPageContext context;
11+
protected readonly IContentPageContext ContentPageContext = Ioc.Default.GetRequiredService<IContentPageContext>();
12+
protected readonly IWindowsWallpaperService WindowsWallpaperService = Ioc.Default.GetRequiredService<IWindowsWallpaperService>();
1213

1314
public abstract string Label { get; }
1415

@@ -17,16 +18,14 @@ internal abstract class BaseSetAsAction : ObservableObject, IAction
1718
public abstract RichGlyph Glyph { get; }
1819

1920
public virtual bool IsExecutable =>
20-
context.ShellPage is not null &&
21-
context.PageType != ContentPageTypes.RecycleBin &&
22-
context.PageType != ContentPageTypes.ZipFolder &&
23-
(context.ShellPage?.SlimContentPage?.SelectedItemsPropertiesViewModel?.IsCompatibleToSetAsWindowsWallpaper ?? false);
21+
ContentPageContext.ShellPage is not null &&
22+
ContentPageContext.PageType != ContentPageTypes.RecycleBin &&
23+
ContentPageContext.PageType != ContentPageTypes.ZipFolder &&
24+
(ContentPageContext.ShellPage?.SlimContentPage?.SelectedItemsPropertiesViewModel?.IsCompatibleToSetAsWindowsWallpaper ?? false);
2425

2526
public BaseSetAsAction()
2627
{
27-
context = Ioc.Default.GetRequiredService<IContentPageContext>();
28-
29-
context.PropertyChanged += Context_PropertyChanged;
28+
ContentPageContext.PropertyChanged += ContentPageContext_PropertyChanged;
3029
}
3130

3231
public abstract Task ExecuteAsync(object? parameter = null);
@@ -46,7 +45,7 @@ protected async void ShowErrorDialog(string message)
4645
await errorDialog.TryShowAsync();
4746
}
4847

49-
private void Context_PropertyChanged(object? sender, PropertyChangedEventArgs e)
48+
private void ContentPageContext_PropertyChanged(object? sender, PropertyChangedEventArgs e)
5049
{
5150
switch (e.PropertyName)
5251
{
@@ -56,10 +55,10 @@ private void Context_PropertyChanged(object? sender, PropertyChangedEventArgs e)
5655
case nameof(IContentPageContext.SelectedItem):
5756
case nameof(IContentPageContext.SelectedItems):
5857
{
59-
if (context.ShellPage is not null && context.ShellPage.SlimContentPage is not null)
58+
if (ContentPageContext.ShellPage is not null && ContentPageContext.ShellPage.SlimContentPage is not null)
6059
{
61-
var viewModel = context.ShellPage.SlimContentPage.SelectedItemsPropertiesViewModel;
62-
var extensions = context.SelectedItems.Select(selectedItem => selectedItem.FileExtension).Distinct().ToList();
60+
var viewModel = ContentPageContext.ShellPage.SlimContentPage.SelectedItemsPropertiesViewModel;
61+
var extensions = ContentPageContext.SelectedItems.Select(selectedItem => selectedItem.FileExtension).Distinct().ToList();
6362

6463
viewModel.CheckAllFileExtensions(extensions);
6564
}

‎src/Files.App/Actions/Content/Background/SetAsAppBackgroundAction.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace Files.App.Actions
55
{
66
internal sealed class SetAsAppBackgroundAction : BaseSetAsAction
77
{
8-
private IAppearanceSettingsService AppearanceSettingsService { get; } = Ioc.Default.GetRequiredService<IAppearanceSettingsService>();
8+
private readonly IAppearanceSettingsService AppearanceSettingsService = Ioc.Default.GetRequiredService<IAppearanceSettingsService>();
99

1010
public override string Label
1111
=> "SetAsAppBackground".GetLocalizedResource();
@@ -18,12 +18,12 @@ public override RichGlyph Glyph
1818

1919
public override bool IsExecutable =>
2020
base.IsExecutable &&
21-
context.SelectedItem is not null;
21+
ContentPageContext.SelectedItem is not null;
2222

2323
public override Task ExecuteAsync(object? parameter = null)
2424
{
25-
if (context.SelectedItem is not null)
26-
AppearanceSettingsService.AppThemeBackgroundImageSource = context.SelectedItem.ItemPath;
25+
if (IsExecutable && ContentPageContext.SelectedItem is ListedItem selectedItem)
26+
AppearanceSettingsService.AppThemeBackgroundImageSource = selectedItem.ItemPath;
2727

2828
return Task.CompletedTask;
2929
}

‎src/Files.App/Actions/Content/Background/SetAsLockscreenBackgroundAction.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) 2024 Files Community
22
// Licensed under the MIT License. See the LICENSE.
33

4+
using Microsoft.Extensions.Logging;
5+
46
namespace Files.App.Actions
57
{
68
internal sealed class SetAsLockscreenBackgroundAction : BaseSetAsAction
@@ -18,16 +20,16 @@ public override RichGlyph Glyph
1820

1921
public override bool IsExecutable =>
2022
base.IsExecutable &&
21-
context.SelectedItem is not null;
23+
ContentPageContext.SelectedItem is not null;
2224

2325
public override Task ExecuteAsync(object? parameter = null)
2426
{
25-
if (context.SelectedItem is null)
27+
if (!IsExecutable || ContentPageContext.SelectedItem is not ListedItem selectedItem)
2628
return Task.CompletedTask;
2729

2830
try
2931
{
30-
return WindowsWallpaperService.SetLockScreenWallpaper(context.SelectedItem.ItemPath);
32+
return WindowsWallpaperService.SetLockScreenWallpaper(selectedItem.ItemPath);
3133
}
3234
catch (Exception ex)
3335
{

‎src/Files.App/Actions/Content/Background/SetAsSlideshowBackgroundAction.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) 2024 Files Community
22
// Licensed under the MIT License. See the LICENSE.
33

4+
using Microsoft.Extensions.Logging;
5+
46
namespace Files.App.Actions
57
{
68
internal sealed class SetAsSlideshowBackgroundAction : BaseSetAsAction
@@ -18,11 +20,12 @@ public override RichGlyph Glyph
1820

1921
public override bool IsExecutable =>
2022
base.IsExecutable &&
21-
context.SelectedItems.Count > 1;
23+
ContentPageContext.SelectedItems.Count > 1;
2224

2325
public override Task ExecuteAsync(object? parameter = null)
2426
{
25-
var paths = context.SelectedItems.Select(item => item.ItemPath).ToArray();
27+
if (!IsExecutable || ContentPageContext.SelectedItems.Select(item => item.ItemPath).ToArray() is not string[] paths || paths.Length is 0)
28+
return Task.CompletedTask;
2629

2730
try
2831
{

‎src/Files.App/Actions/Content/Background/SetAsWallpaperBackgroundAction.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
// Copyright (c) 2024 Files Community
22
// Licensed under the MIT License. See the LICENSE.
33

4+
using Microsoft.Extensions.Logging;
5+
46
namespace Files.App.Actions
57
{
68
internal sealed class SetAsWallpaperBackgroundAction : BaseSetAsAction
79
{
8-
private readonly IWindowsWallpaperService WindowsWallpaperService = Ioc.Default.GetRequiredService<IWindowsWallpaperService>();
9-
1010
public override string Label
1111
=> "SetAsBackground".GetLocalizedResource();
1212

@@ -18,16 +18,16 @@ public override RichGlyph Glyph
1818

1919
public override bool IsExecutable =>
2020
base.IsExecutable &&
21-
context.SelectedItem is not null;
21+
ContentPageContext.SelectedItem is not null;
2222

2323
public override Task ExecuteAsync(object? parameter = null)
2424
{
25-
if (context.SelectedItem is null)
25+
if (!IsExecutable || ContentPageContext.SelectedItem is not ListedItem selectedItem)
2626
return Task.CompletedTask;
2727

2828
try
2929
{
30-
WindowsWallpaperService.SetDesktopWallpaper(context.SelectedItem.ItemPath);
30+
WindowsWallpaperService.SetDesktopWallpaper(selectedItem.ItemPath);
3131
}
3232
catch (Exception ex)
3333
{

‎src/Files.App/Data/Contracts/IWindowsWallpaperService.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,21 @@ public interface IWindowsWallpaperService
1212
/// Sets desktop wallpaper using the specified image path.
1313
/// </summary>
1414
/// <param name="szPath">The image path to assign as the desktop wallpaper.</param>
15+
/// <exception cref="System.Runtime.InteropServices.COMException">Thrown if any of COM methods failed to process successfully.</exception>
1516
void SetDesktopWallpaper(string szPath);
1617

1718
/// <summary>
1819
/// Sets desktop slideshow using the specified image paths.
1920
/// </summary>
2021
/// <param name="aszPaths">The image paths to use to set as slideshow.</param>
22+
/// <exception cref="System.Runtime.InteropServices.COMException">Thrown if any of COM methods failed to process successfully.</exception>
2123
void SetDesktopSlideshow(string[] aszPaths);
2224

2325
/// <summary>
2426
/// Gets lock screen wallpaper using the specified image path.
2527
/// </summary>
2628
/// <param name="szPath">The image path to use to set as lock screen wallpaper.</param>
29+
/// <exception cref="System.Runtime.InteropServices.COMException">Thrown if any of COM methods failed to process successfully.</exception>
2730
Task SetLockScreenWallpaper(string szPath);
2831
}
2932
}

‎src/Files.App/Services/Windows/WindowsWallpaperService.cs

Lines changed: 28 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -17,68 +17,54 @@ public sealed class WindowsWallpaperService : IWindowsWallpaperService
1717
/// <inheritdoc/>
1818
public unsafe void SetDesktopWallpaper(string szPath)
1919
{
20+
// Instantiate IDesktopWallpaper
2021
using ComPtr<IDesktopWallpaper> pDesktopWallpaper = default;
21-
var desktopWallpaperIid = typeof(IDesktopWallpaper).GUID;
22-
var desktopWallpaperInstanceIid = typeof(DesktopWallpaper).GUID;
22+
HRESULT hr = pDesktopWallpaper.CoCreateInstance<DesktopWallpaper>().ThrowOnFailure();
2323

24-
PInvoke.CoCreateInstance(
25-
&desktopWallpaperInstanceIid,
26-
null,
27-
CLSCTX.CLSCTX_LOCAL_SERVER,
28-
&desktopWallpaperIid,
29-
(void**)pDesktopWallpaper.GetAddressOf())
30-
.ThrowOnFailure();
31-
32-
pDesktopWallpaper.Get()->GetMonitorDevicePathCount(out var dwMonitorCount);
24+
// Get total count of all available monitors
25+
hr = pDesktopWallpaper.Get()->GetMonitorDevicePathCount(out var dwMonitorCount).ThrowOnFailure();
3326

3427
fixed (char* pszPath = szPath)
3528
{
36-
PWSTR pMonitorID = default;
29+
PWSTR pMonitorId = default;
3730

38-
for (uint dwIndex = 0; dwIndex < dwMonitorCount; dwIndex++)
31+
// Set the selected image file as wallpaper for all available monitors
32+
for (uint dwIndex = 0u; dwIndex < dwMonitorCount; dwIndex++)
3933
{
40-
pDesktopWallpaper.Get()->GetMonitorDevicePathAt(dwIndex, &pMonitorID);
41-
pDesktopWallpaper.Get()->SetWallpaper(pMonitorID, pszPath);
42-
pMonitorID = default;
34+
// Set the wallpaper
35+
hr = pDesktopWallpaper.Get()->GetMonitorDevicePathAt(dwIndex, &pMonitorId).ThrowOnFailure();
36+
hr = pDesktopWallpaper.Get()->SetWallpaper(pMonitorId, pszPath).ThrowOnFailure();
37+
38+
pMonitorId = default;
4339
}
4440
}
4541
}
4642

4743
/// <inheritdoc/>
4844
public unsafe void SetDesktopSlideshow(string[] aszPaths)
4945
{
46+
// Instantiate IDesktopWallpaper
5047
using ComPtr<IDesktopWallpaper> pDesktopWallpaper = default;
51-
var desktopWallpaperIid = typeof(IDesktopWallpaper).GUID;
52-
var desktopWallpaperInstanceIid = typeof(DesktopWallpaper).GUID;
48+
HRESULT hr = pDesktopWallpaper.CoCreateInstance<DesktopWallpaper>().ThrowOnFailure();
5349

54-
PInvoke.CoCreateInstance(
55-
&desktopWallpaperInstanceIid,
56-
null,
57-
CLSCTX.CLSCTX_LOCAL_SERVER,
58-
&desktopWallpaperIid,
59-
(void**)pDesktopWallpaper.GetAddressOf())
60-
.ThrowOnFailure();
50+
uint dwCount = (uint)aszPaths.Length;
51+
ITEMIDLIST** ppItemIdList = stackalloc ITEMIDLIST*[aszPaths.Length];
6152

62-
var dwCount = (uint)aszPaths.Length;
53+
// Get an array of PIDL from the selected image files
54+
for (uint dwIndex = 0u; dwIndex < dwCount; dwIndex++)
55+
ppItemIdList[dwIndex] = PInvoke.ILCreateFromPath(aszPaths[dwIndex]);
6356

64-
fixed (ITEMIDLIST** idList = new ITEMIDLIST*[dwCount])
65-
{
66-
for (uint dwIndex = 0u; dwIndex < dwCount; dwIndex++)
67-
{
68-
var id = PInvoke.ILCreateFromPath(aszPaths[dwIndex]);
69-
idList[dwIndex] = id;
70-
}
71-
72-
// Get shell item array from images to use for slideshow
73-
using ComPtr<IShellItemArray> pShellItemArray = default;
74-
PInvoke.SHCreateShellItemArrayFromIDLists(dwCount, idList, pShellItemArray.GetAddressOf());
57+
// Get an IShellItemArray from the array of the PIDL
58+
using ComPtr<IShellItemArray> pShellItemArray = default;
59+
hr = PInvoke.SHCreateShellItemArrayFromIDLists(dwCount, ppItemIdList, pShellItemArray.GetAddressOf()).ThrowOnFailure();
7560

76-
// Set slideshow
77-
pDesktopWallpaper.Get()->SetSlideshow(pShellItemArray.Get());
78-
}
61+
// Release the allocated PIDL
62+
for (uint dwIndex = 0u; dwIndex < dwCount; dwIndex++)
63+
PInvoke.CoTaskMemFree((void*)ppItemIdList[dwIndex]);
7964

80-
// Set wallpaper to fill desktop.
81-
pDesktopWallpaper.Get()->SetPosition(DESKTOP_WALLPAPER_POSITION.DWPOS_FILL);
65+
// Set the slideshow and its position
66+
hr = pDesktopWallpaper.Get()->SetSlideshow(pShellItemArray.Get()).ThrowOnFailure();
67+
hr = pDesktopWallpaper.Get()->SetPosition(DESKTOP_WALLPAPER_POSITION.DWPOS_FILL).ThrowOnFailure();
8268
}
8369

8470
/// <inheritdoc/>

0 commit comments

Comments
 (0)