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

filters is very slow #1804

Open
quarksb opened this issue Aug 15, 2024 · 8 comments · May be fixed by #1815
Open

filters is very slow #1804

quarksb opened this issue Aug 15, 2024 · 8 comments · May be fixed by #1815

Comments

@quarksb
Copy link

quarksb commented Aug 15, 2024

the api getImageData is very slow beacause it need gpu convert data to cpu, why not just set the filterContext as the input of filters? which could get filter canvas easily

@lavrton
Copy link
Member

lavrton commented Aug 16, 2024

If you mean this https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/filter, it is not supported by Safari. For rendering consistency, I am not sure if I want to do a "fallback" implementation.

@quarksb
Copy link
Author

quarksb commented Aug 16, 2024

actually, i can use canvas globalCompositeOperation or webgl to render my own filter, such as an inner shadow function as shown below, but getImageData and putImageData is really a very slow api. and cost most of the time could you change getImageData as an optional choice? if you want, i can help you change the code and submit pr

export function drawInnerShadow(ctx: OffscreenCanvasRenderingContext2D | CanvasRenderingContext2D, shadowOption: ShadowOption) {
    const { width, height } = ctx.canvas;
    const { offsetX, offsetY, blur, color } = shadowOption;

    const contourCanvas = new OffscreenCanvas(width, height);
    const contourCtx = contourCanvas.getContext('2d')!;

    // 先反向填充原图像
    contourCtx.drawImage(ctx.canvas, 0, 0);
    contourCtx.globalCompositeOperation = "source-out";
    contourCtx.fillRect(0, 0, width, height);

    const shadowCanvas = new OffscreenCanvas(width, height);
    const shadowCtx = shadowCanvas.getContext('2d')!;
    // 再用阴影填充
    shadowCtx.shadowOffsetX = offsetX;
    shadowCtx.shadowOffsetY = offsetY;
    shadowCtx.shadowBlur = blur;
    shadowCtx.shadowColor = color;
    shadowCtx.drawImage(contourCanvas, 0, 0);


    // console.log('drawInnerShadow', shadowOption);

    // 再用原图扣掉阴影
    shadowCtx.globalCompositeOperation = "destination-in";
    shadowCtx.drawImage(ctx.canvas, 0, 0);

    return shadowCanvas;
}

@lavrton
Copy link
Member

lavrton commented Aug 16, 2024

What are you trying to do? Why don't you use regular shadows?

@quarksb
Copy link
Author

quarksb commented Aug 19, 2024

there are two different questions:
"What are you trying to do", I want rmI just want the filter api become faster, like this pull request #1806
"Why don't you use regular shadows" because there isn't inner shadow in regular shadow

@quarksb
Copy link
Author

quarksb commented Sep 3, 2024

@lavrton maybe the new pr is acceptable ? which have no effect on the previous code.

@lavrton
Copy link
Member

lavrton commented Sep 16, 2024

I still don't full get the use case. How slow it is to use the current approach with imageData? Do you change shadows very frequently?

@quarksb
Copy link
Author

quarksb commented Oct 17, 2024

shadows isn't the key, what I mean is, we should use getImageData() as little as possible, becasuse You have to wait a lot of time due to it need to Wait for the gpu to transfer the image data to the cpu, same as putImageData, which need to transfer data from cpu to gpu.
I think I didn't to prove my opinion, this is a consensus
https://stackoverflow.com/questions/3952856/why-is-putimagedata-so-slow

@lavrton
Copy link
Member

lavrton commented Oct 17, 2024

I am very happy to not use getImageData() if:

  1. We can maintain support for current filters.
  2. We can ensure backward compatibility in the API.

The use case with shadow looks too specific for me right now. It can be simpler to just draw into external canvas element and then use it for Konva.Image instance.

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

Successfully merging a pull request may close this issue.

2 participants