-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Windows] Handle non-seekable streams in PlatformImage.FromStream
#23626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Windows] Handle non-seekable streams in PlatformImage.FromStream
#23626
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). |
| using MemoryStream memoryStream = new(); | ||
| stream.CopyTo(memoryStream); | ||
|
|
||
| global::Windows.Foundation.IAsyncOperation<CanvasBitmap> bitmapAsync = CanvasBitmap.LoadAsync(creator, memoryStream.AsRandomAccessStream()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if you have to first reset the memory stream position to 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right, it's documented: https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.copyto?view=net-8.0#definition and I verified it experimentally.
Fixed by adding the third commit.
e58d2fd to
4977835
Compare
4977835 to
655f806
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
@mattleibow Could you please take a look? |
|
Closing in favor of #23824. |
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:
Issues Fixed
Fixes #18954