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

Optimisation Required / Feature Request #395

Open
Stuart88 opened this issue Dec 13, 2024 · 3 comments
Open

Optimisation Required / Feature Request #395

Stuart88 opened this issue Dec 13, 2024 · 3 comments
Assignees

Comments

@Stuart88
Copy link

Stuart88 commented Dec 13, 2024

Hi, sorry this post has become very large, but I've spent all evening digging into this and would love to see improvements in this brilliantly useful library.

My issue mostly pertains to the fact that the library appears to not use byte arrays for images, and instead handles things mostly using data urls i.e. throwing lots of very large base64 strings around.

I apologise if I'm wrong with that analysis - I have looked into a lot of the code, and have been trying a lot of things with it in my own project, but it seems to have optimisation issues that I was unable to resolve with any of the existing Cropper library methods.

So this is what I've noticed / would like to suggest:

GetCroppedCanvasDataURLAsync() is far too slow.

Tested with image size >2MB, dimensions 1900 x 700

Not sure what's causing this. I've found the window.getPolygonImage JS method which appears to be where the work is done. Something there is presumably in need of optimisation.

Currently I'm using my own code to handle the raw byte data, and it is a LOT faster.

It takes the raw image bytes, along with Cropper.Blazor.Models.CropBoxData and Cropper.Blazor.Models.ContainerData, then crops the image almost instantly.

export async function cropImage(
    imageBytes: Uint8Array,
    cropBox: CropBoxData,
    container: ContainerData
): Promise<Uint8Array> {

    try {
        const blob = new Blob([imageBytes], { type: "image/png" });
        const imageUrl = URL.createObjectURL(blob);

        const img = new Image();
        img.src = imageUrl;

        await new Promise((resolve, reject) => {
            img.onload = resolve;
            img.onerror = reject;
        });

        const croppingCanvas = document.createElement("canvas");
        const context = croppingCanvas.getContext("2d");

        if (!context) {
            throw new Error("Unable to get canvas context");
        }

        // Calculate uniform scale factor based on image fit within container
        const scale = Math.min(container.width / img.naturalWidth, container.height / img.naturalHeight);

        // Calculate the offsets for centering the image within the container
        const offsetX = (container.width - img.naturalWidth * scale) / 2;
        const offsetY = (container.height - img.naturalHeight * scale) / 2;

        // Adjust crop box coordinates from container space to image space
        const imageCropBox = {
            left: (cropBox.left - offsetX) / scale,
            top: (cropBox.top - offsetY) / scale,
            width: cropBox.width / scale,
            height: cropBox.height / scale,
        };

        // Set the cropping canvas size to match the cropped region
        croppingCanvas.width = imageCropBox.width;
        croppingCanvas.height = imageCropBox.height;

        // Draw the cropped portion of the image onto the cropping canvas
        context.drawImage(
            img,
            imageCropBox.left,
            imageCropBox.top,
            imageCropBox.width,
            imageCropBox.height,
            0,
            0,
            imageCropBox.width,
            imageCropBox.height
        );

        const resizedBlob = await new Promise<Blob | null>((resolve) => {
            croppingCanvas?.toBlob(
                (blob) => resolve(blob),
                'image/jpeg',
                1
            );
        });

        if (resizedBlob) {
            return new Uint8Array(await resizedBlob.arrayBuffer());
        }
        else {
            throw new Error("Resized blob empty");
        }
    } catch (error) {
        console.error("Error cropping image:", error);
        return new Uint8Array();
    }
}

Return image as a byte array

To accompany the above, it would be good to have a C# method in the library that returns the canvas data as a Uint8Array object so it can be handled as a byte array in C#.

I've tried searching but can't find one.

So currently I'm using my own custom code for this too, i.e.

public async Task<byte[]> CropImage(byte[] image, CropBoxData cropBox, ContainerData container)
{
    var module = await moduleTask.Value;
    return await module.InvokeAsync<byte[]>("cropImage", image, cropBox, container);
}

GetCroppedCanvasDataURLAsync() locks the UI.

This even happens in the demo page so I'm sure it's not just an issue I'm facing.

Would it be possible to perform the image processing in the the background? i.e. not any await on the main function, then use DotNet.invokeMethodAsync to send the completed data back.

You could expose an event handler service for this so users can handle it without locking the UI, i.e.

// Cropper JsInterop

async function getCroppedCanvasDataURL() : Promise<void> {
    const dataUrl = await doStufftoGetDataUrl();
    DotNet.invokeMethodAsync('[Cropper Assembly Name]', 'DataUrlProcessingComplete', dataUrl );
}
// A Cropper message handler

public class CropperService : ICropperService
{
    private event Action<string> OnDataUrlProcessingComplete;

    [JSInvokable]
    public static async Task DataUrlProcessingComplete(string dataUrl)
    {
        this.OnDataUrlProcessingComplete?.Invoke(dataUrl);
    }
}
/// MyComponent.razor

[Inject]
public ICropperService CropperService { get; set; }

protect override OnInitalized()
{
    CropperService.OnDataUrlProcessingComplete += DoStuffWithDataUrl;
    CropperService.OnImageBytesProcessingComplete += DoStuffWithImageBytes;
}

Task DoStuffWithDataUrl(string dataUrl)
{
   // stuff
}

Task DoStuffWithImageBytes(byte[] imageBytes)
{
    // stuff
}

Thanks! And sorry for the wall of text, and sorry if I've missed something and all of my commentary is incorrect!

@MaxymGorn
Copy link
Member

Hi @Stuart88, I'll look into your approach, thanks!

@MaxymGorn MaxymGorn self-assigned this Jan 15, 2025
@MaxymGorn
Copy link
Member

MaxymGorn commented Jan 15, 2025

@Stuart88 Have you ever wondered how to handle a case where the image is still being prepared in 'getCroppedCanvasDataURL' method but the cropper component has been destroyed?

@Stuart88
Copy link
Author

@Stuart88 Have you ever wondered how to handle a case where the image is still being prepared in 'getCroppedCanvasDataURL' method but the cropper component has been destroyed?

I haven't looked back at the Cropper code in my project since I made this post, so I don't have any specific thoughts sorry!

However in general I think a way to prevent data being lost if the component is destroyed, is to use a separate service (probably a singleton) to hold onto any data so it can persist a little longer if needed.

Or maybe if the component implements IDisposableAsync it might be possible to prevent disposal until the canvas data processing is complete. I'm not 100% sure about it though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants