Build the dust masks per view, not once for the app - #3413
Open
FrayxRulez wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported by crash telemetry on 12.10.2.0.
Cause
CompositionDustLayers._maskswasstatic, so one set ofCompositionSurfaceBrushserved thewhole app. They are built in
PrepareAsyncfromBootStrapper.Current.Compositor, which isWindow.Current.Compositor— a per-view object. The firstChatViewto be constructed decideswhich view's compositor the masks belong to, and that is the main window.
Open a chat in its own window (
createNewWindow, which goes throughCoreApplication.CreateNewView) and itsChatViewgets a compositor of its own.Buildthencreates the mask brush on that compositor and assigns a mask that belongs to another one, and
composition rejects it. Deleting a message there — or deleting the chat, which flushes the whole
history through
SynchronizedList— kills the window every time.The validation message does not name the compositor, but a wrong type is not reachable: the
only thing ever assigned is
compositor.CreateSurfaceBrush(...), whichMaskaccepts, and thearray is fully populated before it is published, so no element can be null. What is left is the
object's origin.
Fix
Split the two halves that were sharing a lifetime.
The expensive part — the scatter pass over the noise and the PNG encode — has nothing to do with
any compositor, so it stays shared:
EncodeAsync(count)produces the streams once per session(behind a lock, since views are threads and two can ask at once) and hands the same array to
every caller.
The brushes are now
[ThreadStatic], which under UWP is one set per view, and each view loadsthem from a
CloneStream()of the shared stream so two views loading at the same time do notmove each other's cursor.
Not built or run; the file parses clean under Roslyn.
Adjacent and left alone:
CompositionDustVisual._randomis a sharedRandomread fromBuildon every view's thread. It could not be reached from a second view before this change; now it
can, and concurrent bursts in two windows could scramble it. The failure is cosmetic rather than
a crash, so it is not folded in here.