Skip to content
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

Avoid heap corruption issue by setting the property via the DP instead of the property setter #5781

Merged
merged 3 commits into from
Sep 10, 2021
Merged

Avoid heap corruption issue by setting the property via the DP instead of the property setter #5781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a797c12
Avoid heap corruption issue by setting the property via the DP instea…
StephenLPeters Aug 27, 2021
84c74bd
Disable setting the property on 19h1 and 19h2
StephenLPeters Aug 27, 2021
6eb3f18
reenable set by DP to test with repo project
StephenLPeters Sep 8, 2021
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
10 changes: 10 additions & 0 deletions dev/dll/SharedHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ bool SharedHelpers::Is21H1OrHigher()
return IsAPIContractV14Available();
}

bool SharedHelpers::Is20H1OrHigher()
{
return IsAPIContractV10Available();
}

bool SharedHelpers::IsVanadiumOrHigher()
{
return IsAPIContractV9Available();
Expand Down Expand Up @@ -239,6 +244,11 @@ bool SharedHelpers::IsAPIContractV14Available()
return IsAPIContractVxAvailable<14>();
}

bool SharedHelpers::IsAPIContractV10Available()
{
return IsAPIContractVxAvailable<10>();
}

bool SharedHelpers::IsAPIContractV9Available()
{
return IsAPIContractVxAvailable<9>();
Expand Down
54 changes: 34 additions & 20 deletions dev/dll/XamlControlsResources.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,43 +158,43 @@ void XamlControlsResources::UpdateAcrylicBrushesLightTheme(const winrt::IInspect
const auto dictionary = themeDictionary.try_as<winrt::ResourceDictionary>();
if (const auto acrylicBackgroundFillColorDefaultBrush = dictionary.Lookup(box_value(c_AcrylicBackgroundFillColorDefaultBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicBackgroundFillColorDefaultBrush.TintLuminosityOpacity(0.85);
UpdateTintLuminosityOpacity(acrylicBackgroundFillColorDefaultBrush, 0.85);
}
if (const auto acrylicInAppFillColorDefaultBrush = dictionary.Lookup(box_value(c_AcrylicInAppFillColorDefaultBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicInAppFillColorDefaultBrush.TintLuminosityOpacity(0.85);
UpdateTintLuminosityOpacity(acrylicInAppFillColorDefaultBrush, 0.85);
}
if (const auto acrylicBackgroundFillColorDefaultInverseBrush = dictionary.Lookup(box_value(c_AcrylicBackgroundFillColorDefaultInverseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicBackgroundFillColorDefaultInverseBrush.TintLuminosityOpacity(0.96);
UpdateTintLuminosityOpacity(acrylicBackgroundFillColorDefaultInverseBrush, 0.96);
}
if (const auto acrylicInAppFillColorDefaultInverseBrush = dictionary.Lookup(box_value(c_AcrylicInAppFillColorDefaultInverseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicInAppFillColorDefaultInverseBrush.TintLuminosityOpacity(0.96);
UpdateTintLuminosityOpacity(acrylicInAppFillColorDefaultInverseBrush, 0.96);
}
if (const auto acrylicBackgroundFillColorBaseBrush = dictionary.Lookup(box_value(c_AcrylicBackgroundFillColorBaseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicBackgroundFillColorBaseBrush.TintLuminosityOpacity(0.9);
UpdateTintLuminosityOpacity(acrylicBackgroundFillColorBaseBrush, 0.9);
}
if (const auto acrylicInAppFillColorBaseBrush = dictionary.Lookup(box_value(c_AcrylicInAppFillColorBaseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicInAppFillColorBaseBrush.TintLuminosityOpacity(0.9);
UpdateTintLuminosityOpacity(acrylicInAppFillColorBaseBrush, 0.9);
}
if (const auto accentAcrylicBackgroundFillColorDefaultBrush = dictionary.Lookup(box_value(c_AccentAcrylicBackgroundFillColorDefaultBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
accentAcrylicBackgroundFillColorDefaultBrush.TintLuminosityOpacity(0.9);
UpdateTintLuminosityOpacity(accentAcrylicBackgroundFillColorDefaultBrush, 0.9);
}
if (const auto accentAcrylicInAppFillColorDefaultBrush = dictionary.Lookup(box_value(c_AccentAcrylicInAppFillColorDefaultBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
accentAcrylicInAppFillColorDefaultBrush.TintLuminosityOpacity(0.9);
UpdateTintLuminosityOpacity(accentAcrylicInAppFillColorDefaultBrush, 0.9);
}
if (const auto accentAcrylicBackgroundFillColorBaseBrush = dictionary.Lookup(box_value(c_AccentAcrylicBackgroundFillColorBaseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
accentAcrylicBackgroundFillColorBaseBrush.TintLuminosityOpacity(0.9);
UpdateTintLuminosityOpacity(accentAcrylicBackgroundFillColorBaseBrush, 0.9);
}
if (const auto accentAcrylicInAppFillColorBaseBrush = dictionary.Lookup(box_value(c_AccentAcrylicInAppFillColorBaseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
accentAcrylicInAppFillColorBaseBrush.TintLuminosityOpacity(0.9);
UpdateTintLuminosityOpacity(accentAcrylicInAppFillColorBaseBrush, 0.9);
}
}

Expand All @@ -204,47 +204,61 @@ void XamlControlsResources::UpdateAcrylicBrushesDarkTheme(const winrt::IInspecta
{
if (const auto acrylicBackgroundFillColorDefaultBrush = dictionary.Lookup(box_value(c_AcrylicBackgroundFillColorDefaultBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicBackgroundFillColorDefaultBrush.TintLuminosityOpacity(0.96);
UpdateTintLuminosityOpacity(acrylicBackgroundFillColorDefaultBrush, 0.96);
}
if (const auto acrylicInAppFillColorDefaultBrush = dictionary.Lookup(box_value(c_AcrylicInAppFillColorDefaultBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicInAppFillColorDefaultBrush.TintLuminosityOpacity(0.96);
UpdateTintLuminosityOpacity(acrylicInAppFillColorDefaultBrush, 0.96);
}
if (const auto acrylicBackgroundFillColorDefaultInverseBrush = dictionary.Lookup(box_value(c_AcrylicBackgroundFillColorDefaultInverseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicBackgroundFillColorDefaultInverseBrush.TintLuminosityOpacity(0.85);
UpdateTintLuminosityOpacity(acrylicBackgroundFillColorDefaultInverseBrush, 0.85);
}
if (const auto acrylicInAppFillColorDefaultInverseBrush = dictionary.Lookup(box_value(c_AcrylicInAppFillColorDefaultInverseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicInAppFillColorDefaultInverseBrush.TintLuminosityOpacity(0.85);
UpdateTintLuminosityOpacity(acrylicInAppFillColorDefaultInverseBrush, 0.85);
}
if (const auto acrylicBackgroundFillColorBaseBrush = dictionary.Lookup(box_value(c_AcrylicBackgroundFillColorBaseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicBackgroundFillColorBaseBrush.TintLuminosityOpacity(0.96);
UpdateTintLuminosityOpacity(acrylicBackgroundFillColorBaseBrush, 0.96);
}
if (const auto acrylicInAppFillColorBaseBrush = dictionary.Lookup(box_value(c_AcrylicInAppFillColorBaseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
acrylicInAppFillColorBaseBrush.TintLuminosityOpacity(0.96);
UpdateTintLuminosityOpacity(acrylicInAppFillColorBaseBrush, 0.96);
}
if (const auto accentAcrylicBackgroundFillColorDefaultBrush = dictionary.Lookup(box_value(c_AccentAcrylicBackgroundFillColorDefaultBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
accentAcrylicBackgroundFillColorDefaultBrush.TintLuminosityOpacity(0.8);
UpdateTintLuminosityOpacity(accentAcrylicBackgroundFillColorDefaultBrush, 0.8);
}
if (const auto accentAcrylicInAppFillColorDefaultBrush = dictionary.Lookup(box_value(c_AccentAcrylicInAppFillColorDefaultBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
accentAcrylicInAppFillColorDefaultBrush.TintLuminosityOpacity(0.8);
UpdateTintLuminosityOpacity(accentAcrylicInAppFillColorDefaultBrush, 0.8);
}
if (const auto accentAcrylicBackgroundFillColorBaseBrush = dictionary.Lookup(box_value(c_AccentAcrylicBackgroundFillColorBaseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
accentAcrylicBackgroundFillColorBaseBrush.TintLuminosityOpacity(0.8);
UpdateTintLuminosityOpacity(accentAcrylicBackgroundFillColorBaseBrush, 0.8);
}
if (const auto accentAcrylicInAppFillColorBaseBrush = dictionary.Lookup(box_value(c_AccentAcrylicInAppFillColorBaseBrush)).as<winrt::Windows::UI::Xaml::Media::AcrylicBrush>())
{
accentAcrylicInAppFillColorBaseBrush.TintLuminosityOpacity(0.8);
UpdateTintLuminosityOpacity(accentAcrylicInAppFillColorBaseBrush, 0.8);
}
}
}

void XamlControlsResources::UpdateTintLuminosityOpacity(winrt::Windows::UI::Xaml::Media::AcrylicBrush brush, double luminosityValue)
{
if (SharedHelpers::Is20H1OrHigher())
{
brush.TintLuminosityOpacity(luminosityValue);
}
else
{
// On 19h1 and 19h2 there is a heap corruption bug that occurs when setting the TintLuminosityOpacity via the property,
// So we'll set it by the DP instead.
brush.SetValue(winrt::Windows::UI::Xaml::Media::AcrylicBrush::TintLuminosityOpacityProperty(), box_value(luminosityValue));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the only difference is that this doesn't trigger the "notify on property change" handlers, which is probably ok for this prop but would be worth adding a comment for

}
}

void SetDefaultStyleKeyWorker(winrt::IControlProtected const& controlProtected, std::wstring_view const& className)
{
controlProtected.DefaultStyleKey(box_value(className));
Expand Down
1 change: 1 addition & 0 deletions dev/dll/XamlControlsResources.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ class XamlControlsResources :
private:
void UpdateSource();
bool IsControlsResourcesVersion2();
void UpdateTintLuminosityOpacity(winrt::Windows::UI::Xaml::Media::AcrylicBrush brush, double luminosityValue);
};
2 changes: 2 additions & 0 deletions dev/inc/SharedHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class SharedHelpers

// Logical OS version checks
static bool Is21H1OrHigher();
static bool Is20H1OrHigher();
static bool IsVanadiumOrHigher();
static bool Is19H1OrHigher();
static bool IsRS5OrHigher();
Expand Down Expand Up @@ -63,6 +64,7 @@ class SharedHelpers

// Actual OS version checks
static bool IsAPIContractV14Available(); // 21H1
static bool IsAPIContractV10Available(); // 20H1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static bool IsAPIContractV10Available(); // 20H1
static bool IsAPIContractV10Available(); // 20H1

static bool IsAPIContractV9Available(); // 19H2
static bool IsAPIContractV8Available(); // 19H1
static bool IsAPIContractV7Available(); // RS5
Expand Down