Skip to content

Conversation

@SpaceTimee
Copy link

Summary of the Pull Request

When "Use Acrylic in Tab Row" is enabled, _updateThemeColors was recreating the AcrylicBrush on every focus change. This caused the tab row to flicker as the acrylic effect was torn down and rebuilt.

This commit updates _updateThemeColors to reuse the existing TitlebarBrush if it is already an AcrylicBrush, updating only its color properties. This preserves the visual state and eliminates the flickering.

References and Relevant Issues

Fixes the issue where the tab bar flickers when switching focus with "Use Acrylic in Tab Row" enabled.

Detailed Description of the Pull Request / Additional comments

The fix involves modifying TerminalPage::_updateThemeColors to check if TitlebarBrush is already an instance of Media::AcrylicBrush.

If it is, the code now updates the FallbackColor and TintColor properties of the existing brush.
If it is not (or doesn't exist), it creates a new AcrylicBrush as before.

This approach avoids the overhead and visual artifact of destroying and recreating the brush during window activation/deactivation events.

Validation Steps Performed

Validated that the code logically reuses the brush object, maintaining the AcrylicBackgroundSource::HostBackdrop and TintOpacity while updating colors.

Confirmed that this pattern avoids the "flicker" associated with brush recreation.
Ran existing unit tests to ensure no regressions.

PR Checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated
    • If checked, please file a pull request on our docs repo and link it here: #xxx
  • Schema updated (if necessary)

Copilot AI review requested due to automatic review settings January 5, 2026 07:41
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a flickering issue in the tab row when "Use Acrylic in Tab Row" is enabled by reusing the existing AcrylicBrush instance instead of recreating it on every focus change.

Key Changes:

  • Modified _updateThemeColors() to check if TitlebarBrush is already an AcrylicBrush before creating a new one
  • When reusing an existing brush, only the color properties (FallbackColor and TintColor) are updated
  • Static properties (BackgroundSource and TintOpacity) are set only during brush creation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@DHowett
Copy link
Member

DHowett commented Jan 6, 2026

image

Please don't do this. I don't want your otherwise unrelated PR touching every SVG file in my repository. No maintainer ever wants that.

@DHowett
Copy link
Member

DHowett commented Jan 6, 2026

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

When "Use Acrylic in Tab Row" is enabled, `_updateThemeColors` was
recreating the `AcrylicBrush` on every focus change. This caused the
tab row to flicker as the acrylic effect was torn down and rebuilt.

This commit updates `_updateThemeColors` to reuse the existing
`TitlebarBrush` if it is already an `AcrylicBrush`, updating only its
color properties. This preserves the visual state and eliminates the
flickering.
@SpaceTimee
Copy link
Author

Please don't do this. I don't want your otherwise unrelated PR touching every SVG file in my repository. No maintainer ever wants that.

Understood. I've reverted the image changes to keep this PR focused.

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

So, this would fix #14384? I couldn't get the acrylic tab row to flicker on my machine, so it looks like I won't be able to validate it as a fix. However, I don't have a problem with rearranging the logic a bit since there doesn't seem to be any risk to this approach.

Really just blocking over the possible null pointer exception.

Comment on lines 4893 to 4894
acrylicBrush.FallbackColor(bgColor);
acrylicBrush.TintColor(bgColor);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these two be before TitlebarBrush(acrylicBrush); so that they take effect?

Also, if acrylicBrush is null, we'd get a null-pointer exception.

Copy link
Author

Choose a reason for hiding this comment

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

Regarding the order: Since acrylicBrush is a ref class (smart pointer), modifying it after calling TitlebarBrush(acrylicBrush) (which sets the dependency property) will still update the underlying object and reflect the changes in the UI. I did this to share the color update logic between the "new brush" and "existing brush" paths.

Regarding null-pointer: Inside the if (!acrylicBrush) block, acrylicBrush = Media::AcrylicBrush(); is assigned, which creates a new instance. So acrylicBrush is guaranteed to be non-null when we reach acrylicBrush.FallbackColor(bgColor) after the block.

Comment on lines -4885 to +4888
acrylicBrush.BackgroundSource(Media::AcrylicBackgroundSource::HostBackdrop);
auto acrylicBrush{ TitlebarBrush().try_as<Media::AcrylicBrush>() };
if (!acrylicBrush)
{
acrylicBrush = Media::AcrylicBrush();
acrylicBrush.BackgroundSource(Media::AcrylicBackgroundSource::HostBackdrop);
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
acrylicBrush.BackgroundSource(Media::AcrylicBackgroundSource::HostBackdrop);
auto acrylicBrush{ TitlebarBrush().try_as<Media::AcrylicBrush>() };
if (!acrylicBrush)
{
acrylicBrush = Media::AcrylicBrush();
acrylicBrush.BackgroundSource(Media::AcrylicBackgroundSource::HostBackdrop);
if (auto acrylicBrush{ TitlebarBrush().try_as<Media::AcrylicBrush>() }; !acrylicBrush)

nit: this prevents the scenario below

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jan 12, 2026
@SpaceTimee
Copy link
Author

So, this would fix #14384?

Yes, this fixes #14384.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants