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

Muliple instances of cropper in same page lead to performance problems #553

Open
arelsirin1 opened this issue Aug 17, 2023 · 5 comments
Open

Comments

@arelsirin1
Copy link

Hi! first of all thanks for this library it's been pretty useful for us.

We are running into some performance issues when we have multiple croppers in the same page and it makes it difficult to users to be able to crop images.

When there are multiple croppers in the same page, the cropper starts to blink and becomes difficult to manage.
We tried implementing lazy loading with React.lazy(), to see if we could get improvements, but the issue remains.

What would you suggest for us to do to avoid the cropper from blinking?

Attached a video to show blinking example:
blinking cropper.webm

@sekoyo
Copy link
Owner

sekoyo commented Aug 17, 2023

Oh that's interesting, if you inspect the HTML of the blinking one do you see some changes to the DOM flashing? Do you have some global state that is shared between the components that you are setting onChange or something? Although it's strange that I only see 1 of the others blinking 🤔

@sekoyo
Copy link
Owner

sekoyo commented Aug 17, 2023

I made a quick test here and don't have the same issue: https://codesandbox.io/s/react-image-crop-demo-with-react-hooks-forked-znh5zf?file=/src/Cropper.tsx:4656-5414

Screen.Recording.2023-08-17.at.9.10.06.AM.mov

My guess would be that you have some global state which is causing the other components to mount and re-mount

@arelsirin1
Copy link
Author

Hi @dominictobias , thanks for the quick response.
We cannot see HTML DOM flashing, but we see some croppers blinking. It is kind of random behavior, if I start to scroll, some croppers blink other stop blinking, it gives the impression that it is using a lot of memory or processing.

The problem starts when we have more than 30 croppers in the same page.
I will double check the component to see if there is something that is triggering re-render constantly, that could be the issue I agree. Maybe when there are few images the "bug" does not get exposed, but when accumulating several ones, it tells.

I am attaching a new video just in case:
blinking cropper2.webm

However, I will keep digging in the mounting/re-render logic maybe that is the case.
Thanks!

@sekoyo
Copy link
Owner

sekoyo commented Aug 17, 2023

Interesting, I didn't realise you had so many on the page. My guess then would not be on the JS side but on the CSS side, specifically I think the box-shadow which darkens the image could be intensive.

Plz try this experiment:

.ReactCrop__crop-selection {
  box-shadow: none;
}

Of course then you don't have the box shadow so it's hard to test as the flashing will be gone. But it would be interesting if it feels faster, and if a recording of the Performance tab in devtools is better.

You could also turn off the marching ants animation: <ReactCrop className="ReactCrop--no-animate" ... />

There is another technique to darken the image by having two images with a dark layer in between. This most likely uses more memory so I avoided it, but the tradeoff could be worth it if the performance of box-shadow is actually worse than a bit more memory 🤔

There is one other way I know using clip-path #530 but the performance of that was too bad.

@arelsirin1
Copy link
Author

Thanks @dominictobias!
Nice! that caused some improvements, it makes some of the other dom elements (non crop related) blink, but not an issue at this moment. I will do some profiling and provide information later.

I think that additionally there might be some redundant re-rendering related to our custom react logic. I will dig a bit into our code and check that as well, and see what we find there too.

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