[Windows] Handle non-seekable streams in PlatformImage.FromStream (take 2) - #23824
Conversation
|
Hey there @MartyIX! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
jonathanpeppers
left a comment
jonathanpeppers
left a comment
There was a problem hiding this comment.
This looks good to me 👍, I triggered CI.
b41d163 to
45b9ff3
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
mattleibow
left a comment
mattleibow
left a comment
There was a problem hiding this comment.
This is looking good!
Is it possible to create a test for this? You can fake a readonly stream with something like this: https://github.com/mono/SkiaSharp/blob/main/tests/Tests/Utils/NonSeekableReadOnlyStream.cs
Yes, I will try to come with one. |
45b9ff3 to
d1e5a3e
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
6 participants
Alternative to #23626. This PR uses the
RecyclableMemoryStreamManagerto decrease allocations.Description of Change
AsRandomAccessStreamthrowsNotSupportedExceptionwhen the stream is not seekable. This PR is a simple workaround for it.The second commit adds an optimization. The idea is to remove
AsyncPump(introduced in dotnet/Microsoft.Maui.Graphics@4597923 and merged to MAUI in #8739) which does not seem to be necessary at all. I have tested the change with parallel loading of images and it did not break for me. But then again I don't really know whyAsyncPumpwas introduced in the first place. This commit is an alternative to #23624 approach and the improvement should be the same as in that PR: #23624 (comment) (9 seconds -> 1.3 seconds for a batch of images containing ~100 images).Test
Tested with provided sample code that reads images in parallel to make sure it works: https://gist.github.com/MartyIX/2513b1853ad8537a4d31b62c251115ad
Issues Fixed
Fixes #18954