Skip to content
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

How to get perfect square with ImageCropper #4483

Closed
michael-hawker opened this issue Feb 10, 2022 Discussed in #4478 · 5 comments · Fixed by #4720
Closed

How to get perfect square with ImageCropper #4483

michael-hawker opened this issue Feb 10, 2022 Discussed in #4478 · 5 comments · Fixed by #4720
Assignees
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior Completed 🔥 controls 🎛️
Milestone

Comments

@michael-hawker
Copy link
Member

Discussed in #4478

Originally posted by jhariel8 February 7, 2022
Hello,

I'm working on a project in which we need to have an image that is either taken or uploaded by a user cropped to a perfect square. I thought that setting the aspect ratio to 1.0 would accomplish this; however, when I open the picture (a 1920x1080 photo), the crop box fits to the image. When I shrink the crop box, it makes an elongated rectangle instead of a square. Is there a way to make the crop box a rectangle and keep it in that shape?

For reference, here is my XAML:

`

<Grid.RowDefinitions>
    <RowDefinition Height="*"></RowDefinition>
    <RowDefinition Height="Auto"></RowDefinition>
    <RowDefinition Height="70"></RowDefinition>
    <RowDefinition Height="*"></RowDefinition>
</Grid.RowDefinitions>
<Grid.ColumnDefinitions>
    <ColumnDefinition Width="*"></ColumnDefinition>
    <ColumnDefinition Width="600"></ColumnDefinition>
    <ColumnDefinition Width="*"></ColumnDefinition>
</Grid.ColumnDefinitions>
<controls:ImageCropper x:Name="imgCropper" Grid.Row="1" Grid.Column="1" HorizontalAlignment="Stretch" VerticalAlignment="Stretch" Height="{Binding Path=CROPPER_MAX_HEIGHT, ElementName=mainGrid}"></controls:ImageCropper>
<Grid Grid.Row="2" Grid.ColumnSpan="3" Margin="10">
    <Grid.ColumnDefinitions>
        <ColumnDefinition Width="*"></ColumnDefinition>
        <ColumnDefinition Width=".8*"></ColumnDefinition>
        <ColumnDefinition Width=".8*"></ColumnDefinition>
        <ColumnDefinition Width="*"></ColumnDefinition>
    </Grid.ColumnDefinitions>
    <Button x:Name="Cancel" Grid.Column="1" HorizontalAlignment="Stretch" VerticalAlignment="Stretch" Margin="10" Content="{x:Bind Path=i18n:Translator.Instance.GetValue('Cancel'), Mode=OneWay}" Style="{StaticResource BaseButtonUWP}"></Button>
    <Button x:Name="Done" Grid.Column="2" HorizontalAlignment="Stretch" VerticalAlignment="Stretch" Margin="10" Content="{x:Bind Path=i18n:Translator.Instance.GetValue('Done'), Mode=OneWay}" Style="{StaticResource BaseButtonUWP}"></Button>
</Grid>

`

The aspect ratio is set in the code-behind to 1.0.

I would appreciate any thoughts or suggestions!

@ghost ghost added the needs triage 🔍 label Feb 10, 2022
@ghost
Copy link

ghost commented Feb 10, 2022

Hello michael-hawker, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@michael-hawker michael-hawker added this to the 8.0 milestone Feb 10, 2022
@michael-hawker michael-hawker added bug 🐛 An unexpected issue that highlights incorrect behavior controls 🎛️ and removed needs triage 🔍 labels Feb 10, 2022
@ghost ghost added the In-PR 🚀 label Feb 10, 2022
@michael-hawker michael-hawker modified the milestones: 8.0, 7.1.3 Jul 14, 2022
@Arlodotexe
Copy link
Member

Arlodotexe commented Aug 10, 2022

I'll be taking over this issue from here.

I spent some time setting up a minimal repro, and wasn't able to get it to produce the bug.
Going back to the original discussion, I found a repro from the original author.

I'll be updating this comment with my findings as I go.


Looks like the source of the problem is that calling await imgCropper.LoadImageFromFile(file); doesn't wait for the image to load before completing.

var file = await StorageFile.GetFileFromApplicationUriAsync(new Uri("ms-appx:///Assets\\Screenshot 2021-03-16 083652.png"));
await imgCropper.LoadImageFromFile(file);
this.imgCropper.AspectRatio = 1.0;

Adding a Task.Delay() with any value (minimum of 1) fixes the issue.

Notably, this issue also exists when do you

imgCropper.Source = writableBitmap;
imgCropper.AspectRatio = 1;

Additional information

  • When setting the Source property, CanvasRect has a width but no height, causing it to be treated as invalid.
    • Presumably, this is because the image is not yet loaded and the containing Grid row uses Auto for a height.
    • Assigning a Height to the ImageCropper is a workaround (not a fix)

@michael-hawker
Copy link
Member Author

@Arlodotexe the LoadImageFromFile function seems to be awaiting the setsource here:

public async Task LoadImageFromFile(StorageFile imageFile)
{
var writeableBitmap = new WriteableBitmap(1, 1);
using (var stream = await imageFile.OpenReadAsync())
{
await writeableBitmap.SetSourceAsync(stream);
}
Source = writeableBitmap;
}

Basically there's either something that's happening in the XAML system for image loading or our Source changed callback here:

private static void OnSourceChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{
var target = (ImageCropper)d;
if (e.NewValue is WriteableBitmap bitmap)
{
if (bitmap.PixelWidth < target.MinCropSize.Width || bitmap.PixelHeight < target.MinCropSize.Height)
{
target.Source = null;
throw new ArgumentException("The resolution of the image is too small!");
}
}
target.InvalidateMeasure();
target.UpdateCropShape();
target.InitImageLayout();
}

Which we also need to wait on as well, eh?

Do we just need to use a TaskCompletionSource in the LoadImageFromFile method and register to the SourcePropertyChanged, so that we can complete the awaited task for that after that code has run? Then we can of course unregister and continue on?

Would that be sufficient?

@Arlodotexe
Copy link
Member

@michael-hawker I did try adding this code in the OnSourceChanged method:

    var taskCompletionSource = new TaskCompletionSource<object>();

    target._sourceImage.ImageOpened += OnImageOpenedOrFailed;
    target._sourceImage.ImageFailed += OnImageOpenedOrFailed;

    await taskCompletionSource.Task;

    target._sourceImage.ImageOpened -= OnImageOpenedOrFailed;
    target._sourceImage.ImageFailed -= OnImageOpenedOrFailed;

    void OnImageOpenedOrFailed(object sender, RoutedEventArgs e) => taskCompletionSource.SetResult(null);

Which caused some fun side effects that we don't want.

I think the root of the issue is definitely that the AspectRatio is being set while the image is still being loaded. There's a lot of code in UpdateAspectRatio() that is getting skipped and probably not called again once the image is loaded. Investigating if that can help...

@Arlodotexe
Copy link
Member

Arlodotexe commented Aug 10, 2022

!!!

That was it. Calling UpdateAspectRatio(true); inside of ImageCanvas_SizeChanged fixes the issue, that's all it took 🚀

Doing this fixes it for both of these scenarios that were broken:

var file = await StorageFile.GetFileFromApplicationUriAsync(new Uri("ms-appx:///Assets\\Screenshot 2021-03-16 083652.png"));
await imgCropper.LoadImageFromFile(file);

this.imgCropper.AspectRatio = 1.0;
var url = new Uri("http://ipfs.io/ipfs/QmbnXcqg7s5J6wsrP85VkiUdfFnYvpfPGZYRkQ5CteA1BG");
var stream = await RandomAccessStreamReference.CreateFromUri(url).OpenReadAsync();

var writableBitmap = new WriteableBitmap(1000, 1000);
await writableBitmap.SetSourceAsync(stream);

imgCropper.Source = writableBitmap;
imgCropper.AspectRatio = 1;

@ghost ghost added Completed 🔥 and removed In-PR 🚀 labels Sep 29, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.