Skip to content

Commit 347c3f1

Browse files
authored
CmdPal: Enhance font icon classification and visuals (#41573)
## Summary of the Pull Request - Introduces `FontIconGlyphClassifier` for classifying emojis and symbols. - Correctly recognizes multi-codepoint glyphs (e.g., 🧙🏼‍♀️ *woman mage with medium-light skin tone*). - Explicitly disallows multi-glyph icons (they would overflow anyway). - Distinguishes between emojis and regular text characters (letters, numbers, symbols), since emojis are slightly larger and require different padding. - Recognizes Unicode [Variation Selectors](https://en.wikipedia.org/wiki/Variation_Selectors_(Unicode_block)) to enforce specific styles: VS15 (U+FE0E) for text style (monochrome) and VS16 (U+FE0F) for emoji style (color). This lets developers choose which variant to display. By default, characters with both representations render as text/monochrome (e.g., ▶ `\u25B6`): <img width="428" height="39" alt="image" src="https://github.com/user-attachments/assets/c5e6865f-61de-4f45-9f3a-4e15e5e5ceb8" /> - Invalid icons are displayed as a dashed circle so extension developers can spot issues, without being overly distracting if they slip into production. - Updates `IconPathConverter` to use the new classifier for improved icon handling. - Adds `SampleIconPage` to demonstrate various icon usages and classifications. - Adjusts icon alignment in `IconBox` so icons are centered. - Scales negative padding for emojis in `IconBox` with control size, fixing misalignment and clipping (noticeable in tags and the details pane hero image). - Applies negative padding to all font icons. This removes the need for classification in these cases and ensures symbols rendered below the baseline remain visible. Based on [microsoft/terminal#19143](microsoft/terminal#19143): Co-authored-by: Dustin L. Howett <[email protected]> Pictures? Pictures! <img width="1912" height="2394" alt="image" src="https://github.com/user-attachments/assets/05a16309-b658-4f21-8f9d-9a3f20db6ad8" /> Keyboard and flag/country emojis may look a bit off, but that’s how they’re actually rendered: <img width="482" height="95" alt="image" src="https://github.com/user-attachments/assets/dc7d4d0d-3dc8-4df5-9b9f-9e977e7e989f" /> <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: - #41489 - #41496 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed
1 parent b71bbf8 commit 347c3f1

File tree

9 files changed

+531
-50
lines changed

9 files changed

+531
-50
lines changed

.github/actions/spell-check/expect.txt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ CXVIRTUALSCREEN
306306
CYSCREEN
307307
CYSMICON
308308
CYVIRTUALSCREEN
309+
Czechia
309310
cziplib
310311
Dac
311312
dacl
@@ -436,6 +437,7 @@ EDITSHORTCUTS
436437
EDITTEXT
437438
EFile
438439
ekus
440+
emojis
439441
ENABLEDELAYEDEXPANSION
440442
ENABLEDPOPUP
441443
ENABLETAB
@@ -800,6 +802,7 @@ KEYBOARDMANAGEREDITORLIBRARYWRAPPER
800802
keyboardmanagerstate
801803
keyboardmanagerui
802804
keyboardtester
805+
keycap
803806
KEYEVENTF
804807
KEYIMAGE
805808
keynum
@@ -1781,10 +1784,13 @@ UACUI
17811784
UAL
17821785
uap
17831786
UBR
1787+
UBreak
1788+
ubrk
17841789
UCallback
17851790
ucrt
17861791
ucrtd
17871792
uefi
1793+
UError
17881794
uesc
17891795
UFlags
17901796
UHash
@@ -1854,6 +1860,7 @@ VFT
18541860
vget
18551861
vgetq
18561862
viewmodels
1863+
virama
18571864
VIRTKEY
18581865
VIRTUALDESK
18591866
VISEGRADRELAY

src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/IconBox.cs

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
// The Microsoft Corporation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using ManagedCommon;
56
using Microsoft.CmdPal.Core.ViewModels;
67
using Microsoft.CmdPal.UI.Deferred;
8+
using Microsoft.Terminal.UI;
79
using Microsoft.UI.Dispatching;
810
using Microsoft.UI.Xaml;
911
using Microsoft.UI.Xaml.Controls;
@@ -55,6 +57,8 @@ public IconBox()
5557
{
5658
TabFocusNavigation = KeyboardNavigationMode.Once;
5759
IsTabStop = false;
60+
HorizontalContentAlignment = HorizontalAlignment.Center;
61+
VerticalContentAlignment = VerticalAlignment.Center;
5862
}
5963

6064
private static void OnSourcePropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
@@ -75,6 +79,8 @@ private static void OnSourcePropertyChanged(DependencyObject d, DependencyProper
7579
IconSourceElement elem = new()
7680
{
7781
IconSource = fontIco,
82+
HorizontalAlignment = HorizontalAlignment.Center,
83+
VerticalAlignment = VerticalAlignment.Center,
7884
};
7985
@this.Content = elem;
8086
break;
@@ -98,14 +104,20 @@ private static void OnSourceKeyPropertyChanged(DependencyObject d, DependencyPro
98104
else
99105
{
100106
// TODO GH #239 switch back when using the new MD text block
107+
// Switching back to EnqueueAsync has broken icons in tags (they don't show)
101108
// _ = @this._queue.EnqueueAsync(() =>
102-
@this._queue.TryEnqueue(new(async () =>
109+
@this._queue.TryEnqueue(async void () =>
103110
{
104-
var requestedTheme = @this.ActualTheme;
105-
var eventArgs = new SourceRequestedEventArgs(e.NewValue, requestedTheme);
106-
107-
if (@this.SourceRequested is not null)
111+
try
108112
{
113+
if (@this.SourceRequested is null)
114+
{
115+
return;
116+
}
117+
118+
var requestedTheme = @this.ActualTheme;
119+
var eventArgs = new SourceRequestedEventArgs(e.NewValue, requestedTheme);
120+
109121
await @this.SourceRequested.InvokeAsync(@this, eventArgs);
110122

111123
// After the await:
@@ -130,37 +142,35 @@ private static void OnSourceKeyPropertyChanged(DependencyObject d, DependencyPro
130142
// So, if the icon we get back was a font icon,
131143
// and the glyph for that icon is NOT in the range of
132144
// Segoe icons, then let's give the icon some extra space
133-
@this.Padding = new Thickness(0);
134-
135-
IconDataViewModel? iconData = null;
136-
if (eventArgs.Key is IconDataViewModel)
145+
var iconData = eventArgs.Key switch
137146
{
138-
iconData = eventArgs.Key as IconDataViewModel;
139-
}
140-
else if (eventArgs.Key is IconInfoViewModel info)
147+
IconDataViewModel key => key,
148+
IconInfoViewModel info => requestedTheme == ElementTheme.Light ? info.Light : info.Dark,
149+
_ => null,
150+
};
151+
152+
if (iconData?.Icon is not null && @this.Source is FontIconSource)
141153
{
142-
iconData = requestedTheme == ElementTheme.Light ? info.Light : info.Dark;
143-
}
154+
var iconSize =
155+
!double.IsNaN(@this.Width) ? @this.Width :
156+
!double.IsNaN(@this.Height) ? @this.Height :
157+
@this.ActualWidth > 0 ? @this.ActualWidth :
158+
@this.ActualHeight;
144159

145-
if (iconData is not null &&
146-
@this.Source is FontIconSource)
160+
@this.Padding = new Thickness(Math.Round(iconSize * -0.2));
161+
}
162+
else
147163
{
148-
if (!string.IsNullOrEmpty(iconData.Icon) && iconData.Icon.Length <= 2)
149-
{
150-
var ch = iconData.Icon[0];
151-
152-
// The range of MDL2 Icons isn't explicitly defined, but
153-
// we're using this based off the table on:
154-
// https://docs.microsoft.com/en-us/windows/uwp/design/style/segoe-ui-symbol-font
155-
var isMDL2Icon = ch is >= '\uE700' and <= '\uF8FF';
156-
if (!isMDL2Icon)
157-
{
158-
@this.Padding = new Thickness(-4);
159-
}
160-
}
164+
@this.Padding = default;
161165
}
162166
}
163-
}));
167+
catch (Exception ex)
168+
{
169+
// Exception from TryEnqueue bypasses the global error handler,
170+
// and crashes the app.
171+
Logger.LogError("Failed to set icon", ex);
172+
}
173+
});
164174
}
165175
}
166176
}
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
#include "pch.h"
2+
#include "FontIconGlyphClassifier.h"
3+
#include "FontIconGlyphClassifier.g.cpp"
4+
5+
#include <icu.h>
6+
#include <utility>
7+
8+
namespace winrt::Microsoft::Terminal::UI::implementation
9+
{
10+
namespace
11+
{
12+
// Check if the code point is in the Private Use Area range used by Fluent UI icons.
13+
[[nodiscard]] constexpr bool _isFluentIconPua(const UChar32 cp) noexcept
14+
{
15+
static constexpr UChar32 _fluentIconsPrivateUseAreaStart = 0xE700;
16+
static constexpr UChar32 _fluentIconsPrivateUseAreaEnd = 0xF8FF;
17+
return cp >= _fluentIconsPrivateUseAreaStart && cp <= _fluentIconsPrivateUseAreaEnd;
18+
}
19+
20+
// Determine if the given text (as a sequence of UChar code units) is emoji
21+
[[nodiscard]] bool _isEmoji(const UChar* p, const int32_t length) noexcept
22+
{
23+
if (!p || length < 1)
24+
{
25+
return false;
26+
}
27+
28+
// https://www.unicode.org/reports/tr51/#Emoji_Variation_Selector_Notes
29+
constexpr UChar32 vs15CodePoint = 0xFE0E; // Variation Selectors 15: text variation selector
30+
constexpr UChar32 vs16CodePoint = 0xFE0F; // Variation Selectors: 16 emoji variation selector
31+
32+
// Decode the first code point correctly (surrogate-safe)
33+
int32_t i0{ 0 };
34+
UChar32 first{ 0 };
35+
U16_NEXT(p, i0, length, first);
36+
37+
for (int32_t i = 0; i < length;)
38+
{
39+
UChar32 cp{ 0 };
40+
U16_NEXT(p, i, length, cp);
41+
42+
if (cp == vs16CodePoint) { return true; }
43+
if (cp == vs15CodePoint) { return false; }
44+
}
45+
46+
return !U_IS_SURROGATE(first) && u_hasBinaryProperty(first, UCHAR_EMOJI_PRESENTATION);
47+
}
48+
}
49+
50+
bool FontIconGlyphClassifier::IsLikelyToBeEmojiOrSymbolIcon(const hstring& text)
51+
{
52+
if (text.empty())
53+
{
54+
return false;
55+
}
56+
57+
if (text.size() == 1 && !IS_HIGH_SURROGATE(text[0]))
58+
{
59+
// If it's a single code unit, it's definitely either zero or one grapheme clusters.
60+
// If it turns out to be illegal Unicode, we don't really care.
61+
return true;
62+
}
63+
64+
if (text.size() >= 2 && text[0] <= 0x7F && text[1] <= 0x7F)
65+
{
66+
// Two adjacent ASCII characters (as seen in most file paths) aren't a single
67+
// grapheme cluster.
68+
return false;
69+
}
70+
71+
// Use ICU to determine whether text is composed of a single grapheme cluster.
72+
int32_t off{ 0 };
73+
UErrorCode status{ U_ZERO_ERROR };
74+
75+
UBreakIterator* const bi{ ubrk_open(UBRK_CHARACTER,
76+
nullptr,
77+
reinterpret_cast<const UChar*>(text.data()),
78+
static_cast<int>(text.size()),
79+
&status) };
80+
if (bi)
81+
{
82+
if (U_SUCCESS(status))
83+
{
84+
off = ubrk_next(bi);
85+
}
86+
ubrk_close(bi);
87+
}
88+
return std::cmp_equal(off, text.size());
89+
}
90+
91+
FontIconGlyphKind FontIconGlyphClassifier::Classify(hstring const& text) noexcept
92+
{
93+
if (text.empty())
94+
{
95+
return FontIconGlyphKind::None;
96+
}
97+
98+
const size_t textSize{ text.size() };
99+
const auto* buffer{ reinterpret_cast<const UChar*>(text.c_str()) };
100+
101+
// Fast path 1: Single UTF-16 code unit (most common case)
102+
if (textSize == 1)
103+
{
104+
const UChar ch{ buffer[0] };
105+
106+
if (IS_HIGH_SURROGATE(ch))
107+
{
108+
return FontIconGlyphKind::Invalid;
109+
}
110+
111+
if (_isFluentIconPua(ch))
112+
{
113+
return FontIconGlyphKind::FluentSymbol;
114+
}
115+
116+
if (_isEmoji(&ch, 1))
117+
{
118+
return FontIconGlyphKind::Emoji;
119+
}
120+
121+
return FontIconGlyphKind::Other;
122+
}
123+
124+
// Fast path 2: Common file path pattern - two ASCII printable characters
125+
if (textSize >= 2 && buffer[0] <= 0x7F && buffer[1] <= 0x7F)
126+
{
127+
// Definitely multiple graphemes
128+
return FontIconGlyphKind::Invalid;
129+
}
130+
131+
// Expensive path: Use ICU to determine grapheme boundaries
132+
UErrorCode status{ U_ZERO_ERROR };
133+
134+
UBreakIterator* bi{ ubrk_open(UBRK_CHARACTER,
135+
nullptr,
136+
buffer,
137+
static_cast<int32_t>(textSize),
138+
&status) };
139+
140+
if (U_FAILURE(status) || !bi)
141+
{
142+
return FontIconGlyphKind::Invalid;
143+
}
144+
145+
const int32_t start{ ubrk_first(bi) };
146+
const int32_t end{ ubrk_next(bi) }; // end of first grapheme
147+
ubrk_close(bi);
148+
149+
// No graphemes found
150+
if (end == UBRK_DONE || end <= start)
151+
{
152+
return FontIconGlyphKind::None;
153+
}
154+
155+
// If there's more than one grapheme, it's not a valid icon glyph
156+
if (std::cmp_not_equal(end, textSize))
157+
{
158+
return FontIconGlyphKind::Invalid;
159+
}
160+
161+
// Exactly one grapheme: classify
162+
const UChar* grapheme = buffer + start;
163+
const int32_t graphemeLength = end - start;
164+
165+
if (graphemeLength == 1 && _isFluentIconPua(grapheme[0]))
166+
{
167+
return FontIconGlyphKind::FluentSymbol;
168+
}
169+
170+
if (_isEmoji(grapheme, graphemeLength))
171+
{
172+
return FontIconGlyphKind::Emoji;
173+
}
174+
175+
return FontIconGlyphKind::Other;
176+
}
177+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#pragma once
2+
3+
#include "FontIconGlyphClassifier.g.h"
4+
5+
namespace winrt::Microsoft::Terminal::UI::implementation
6+
{
7+
struct FontIconGlyphClassifier
8+
{
9+
[[nodiscard]] static bool IsLikelyToBeEmojiOrSymbolIcon(const winrt::hstring& text);
10+
11+
[[nodiscard]] static FontIconGlyphKind Classify(winrt::hstring const& text) noexcept;
12+
};
13+
}
14+
15+
namespace winrt::Microsoft::Terminal::UI::factory_implementation
16+
{
17+
BASIC_FACTORY(FontIconGlyphClassifier);
18+
}

0 commit comments

Comments
 (0)