Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[UWP] Fix MultiWindow crash using default brushes #14795

Merged
merged 1 commit into from
Oct 27, 2021
Merged

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Oct 26, 2021

Description of Change

Fix MultiWindow crash using default brushes on UWP.

Issues Resolved

API Changes

None

Platforms Affected

  • UWP

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Launch Core Gallery on Windows. Launch several times (more than one time) a new Window (tap the "Open Secondary Window" Button). Without exceptions, the test has passed.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@jfversluis jfversluis merged commit 5e9ddfd into 5.0.0 Oct 27, 2021
@jfversluis jfversluis deleted the fix-14210 branch October 27, 2021 12:40
@KPixel
Copy link

KPixel commented Oct 27, 2021

Can you please explain how this approach works?

It seems to take the same Brush instance, and update its Parent every time the brush is used somewhere. This change triggers a PropertyChanged event that is propagated to the other (past) parents (which are in different still-opened windows) from the "Device Dispatcher thread"...
So, in my app, I still get the exception RPC_E_WRONG_THREAD.

By the way, should this fix be applied to MAUI?
It probably has the same issue:

https://github.com/dotnet/maui/blob/ac11efee51876880e4029f055fdd3976f6a09514/src/Controls/src/Core/VisualElement.cs#L224

@KPixel
Copy link

KPixel commented Oct 27, 2021

For context, the solution that works for me is to change all default brushes.

// From:
public static readonly SolidColorBrush AliceBlue = new SolidColorBrush(Color.AliceBlue);

// To:
[System.ThreadStatic]
private static SolidColorBrush _aliceBlue;
public static SolidColorBrush AliceBlue => _aliceBlue ??= new SolidColorBrush(Color.AliceBlue);

I automated that change with a macro.

Edit: The brush's Parent still gets changed at every use, but it only applies to the same Dispatcher/thread, so no exceptions.

A small upside is that these 141 brushes get instantiated on-demand now, instead of all at once.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Predefined Brushes fail in a multi-window app
3 participants