Repository navigation
Conversation
2 of 5 tasks
2 of 5 tasks
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
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.
GitHub Issue: closes #25866
#25503 makes a similar
CreateFromStreamchange inside the Wayland host work: it seeks each clone to 0, doesn't fall back to the source's content type, and locks the clone copies ofInMemoryRandomAccessStream(unneeded after this PR) andRandomAccessStreamOverStream. This PR is the fix on its own, with runtime tests. Both changeRandomAccessStreamReference.cs,RandomAccessStreamWithContentType.csandInMemoryRandomAccessStream.cs, so the second to merge gets a small conflict; I'll rebase this one if #25503 lands first.PR Type:
🐞 Bugfix
What changed? 🚀
RandomAccessStreamReference.CreateFromStream(stream)returned the caller's stream from everyOpenReadAsync(), so all opens shared one position and one lifetime, and disposing one open disposed the source for every later open. A bitmap pasted from the Windows clipboard, which Uno creates withCreateFromStream, could be opened only once.As on WinUI, each open now clones the source, so it starts at 0 with its own position and lifetime (
CloneStreamdocuments both):InMemoryRandomAccessStream.CloneStreamcopies its buffer, so the clone starts at 0 and the source doesn't move; it used to start the clone at the source's position, moving the source while copying.RandomAccessStreamWithContentType.CloneStreamkeeps the content type, as WinUI's clones do.CreateFromStreamopensstream.CloneStream(), and gives the clone the source's content type when it has none. If the source can't be cloned, the open fails, as on WinUI. Every Uno stream's clone starts at 0, so the open doesn't seek: some Android document-provider streams can't.Win32ClipboardExtension.ResolveBitmapdisposes the stream it opens, now a clone it owns, instead of restoring its position.Threading: concurrent opens of one reference no longer race for an in-memory source. The
AsRandomAccessStream()wrapper still moves the source while it copies a stream other than aMemoryStream, so those can race (#25503's lock there isn't part of this PR); the Windows clipboard and drag-and-drop references wrap aMemoryStream, andClipboard.SetContentreads the bitmap on the caller's thread. Writes to the source during an open can still give a torn copy.Behavior changes:
CloneStream()throws now fails the open.FileStreamor other .NET stream is read fully into memory on each open, and so is a WebAssembly download-picker file, whose content lives outside the WebAssembly heap. Sources over 2 GB fail (RandomAccessStreamOverStream.CloneStream).BitmapSource.SetSource/SetSourceAsync(BitmapImage,WriteableBitmap) clone the stream, so anInMemoryRandomAccessStreamthat isn't at 0 now decodes from 0, as other Uno streams already did;SvgImageSourcealready decoded from 0.Known issues, not changed here:
image/pngfor PNG bytes, an empty string for unknown data); Uno reports the source's content type, orapplication/octet-stream.CloneStream()doesn't start at 0 opens wherever its clone starts.BitmapSource.SetSourceandSvgImageSource.SetSourceAsyncread the stream itself from its current position to its end, leaving it at the end; Uno decodes a copy from 0 and leaves the stream where it was.MemoryStream.AsRandomAccessStream()wrapper (MemoryStream.ToArray()works on a closed stream), file streams on every platform (their clones reopen the file or rent the base stream), andStreamedRandomAccessStream, whichCreateFromUridownloads use. It fails for anInMemoryRandomAccessStream, as on WinUI.FileMode.OpenOrCreate(FileRandomAccessStream.local.cs,FileRandomAccessStream.UIKit.cs), so a moved or deleted file opens as a new, empty file.Tests: on Skia Desktop (Windows), on
masterat 1f2fc30:Given_InMemoryRandomAccessStream.When_CloneStream_Then_Clone_Starts_At_Zero_And_Source_Does_Not_MoveGiven_StorageFile.When_CloneStream_Then_ContentType_Is_KeptWhen_FromStream_Opened_Twice_Then_Opens_Are_IndependentWhen_FromStream_Opened_Stream_Disposed_Then_Source_And_Other_Opens_Stay_Readable, both data rowsObjectDisposedExceptionWhen_FromStream_Source_Disposed_Then_Earlier_Opens_Stay_Readable_And_Later_Opens_FailObjectDisposedExceptionWhen_FromStream_With_ContentType_Then_Opens_And_Clones_Keep_ItWhen_FromStream_Source_Clones_Have_No_ContentType_Then_Opens_Keep_The_Source_ContentType(not on WinUI, which sniffs)When_FromStream_Over_MemoryStream_Then_Opens_Are_Independent_And_Survive_Dispose(not on WinUI, which can't clone that wrapper)When_FromStream_Source_Cannot_Be_Cloned_Then_Open_FailsGiven_Clipboard.When_Clipboard_Bitmap_Reopened_After_Dispose_Then_Reads_All_Of_It(Win32 clipboard paste)The same assertions, except the clipboard test's, pass in a console app against WinUI's
Windows.Storage.Streams(Windows 11 26200, CsWinRT; a temporary.pngfile stands in for thems-appx:asset), except theMemoryStreamtest's, which is why it's excluded on WinUI. Also with the fix:Given_Clipboard,Given_Win32DragDropand the 66Given_RichEditBoxclipboard tests, in a window station with its own clipboard: 73 pass, 5 skipped by platform (before: the new test fails);Given_WriteableBitmap.When_WriteableBitmap_Assigned_With_Data_Presentfails, as it does without this change;Windows_Storage.StreamsandGiven_StorageFile, with all the new tests but the Win32 one: 188 pass, 1 skipped.PR Checklist ✅
Screenshots Compare Test Runresults.