Skip to content

Conversation

@MartyIX
Copy link
Contributor

@MartyIX MartyIX commented Jul 16, 2024

Description of Change

AsRandomAccessStream throws NotSupportedException when 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 why AsyncPump was 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:

using Microsoft.Maui.Graphics.Platform;

namespace Maui.Controls.Sample;

public partial class MainPage : ContentPage
{
	public MainPage()
	{
		InitializeComponent();
	}

	private async void OnCounterClicked(object sender, EventArgs e)
	{
		try
		{
			var imageUrl = "https://cdn-dynmedia-1.microsoft.com/is/image/microsoftcorp/Highlight-Surface-Laptop-AI-7Ed-Sapphire-MC001-3000x1682:VP5-1920x600";
			var stream = await new HttpClient().GetStreamAsync(imageUrl);
			var img = PlatformImage.FromStream(stream);
		}
		catch (Exception ex)
		{
			Console.WriteLine(ex);
		}

	}
}

Issues Fixed

Fixes #18954

@MartyIX MartyIX requested a review from a team as a code owner July 16, 2024 10:59
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Jul 16, 2024
@dotnet-policy-service
Copy link
Contributor

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.

@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

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());
Copy link
Member

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?

Copy link
Contributor Author

@MartyIX MartyIX Jul 18, 2024

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.

@MartyIX MartyIX force-pushed the feature/2024-07-16-PlatformImage.FromStream-seekable branch from e58d2fd to 4977835 Compare July 18, 2024 11:03
@MartyIX MartyIX force-pushed the feature/2024-07-16-PlatformImage.FromStream-seekable branch from 4977835 to 655f806 Compare July 22, 2024 06:40
@MartyIX MartyIX requested review from Foda and removed request for tj-devel709 July 22, 2024 06:44
@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@MartyIX
Copy link
Contributor Author

MartyIX commented Jul 23, 2024

@mattleibow Could you please take a look?

@MartyIX
Copy link
Contributor Author

MartyIX commented Jul 31, 2024

Closing in favor of #23824.

@MartyIX MartyIX closed this Jul 31, 2024
@MartyIX MartyIX deleted the feature/2024-07-16-PlatformImage.FromStream-seekable branch July 31, 2024 07:26
@github-actions github-actions bot locked and limited conversation to collaborators Aug 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

community ✨ Community Contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows] PlatformImage.FromStream crash

3 participants