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

Transform always applied from image center and there aren't any boundaries #540

Open
loiddy opened this issue Dec 14, 2022 · 9 comments
Open
Labels

Comments

@loiddy
Copy link
Contributor

loiddy commented Dec 14, 2022

I've created ways of fixing both these issues. I have questions though, explained below.

Transform always applied from image center

This doesn't matter when both the image and cropper center are aligned. If they're not, it causes awkward behaviour. See pics below.

1
I've zoomed into and moved to Scooby's face (left pic). But I want Scooby to look to the right, so I flipH (right pic). Now I've lost Scooby and must find him again. He's on the other side because the image got flipped from the center.

To prevent this I've made the transform origin always match the center of the cropper. We no longer have to re-find him. So it looks like this:

2

There aren't any boundaries

When moving the image around I can accidentally move it out the container and never get it back again. I'm left with a whole load of "white space"/ nothingness.

To prevent this I copied Andoriod's WhatsApp and Image Editor. The bits of image outside of the container are "pushed" into the container when the cropper "goes past one of its boundaries". The cropper never really goes past one of the boundaries, the code already implements that (yey, thanks!). But we can see when, where and how much it's doing it and we can use this info to push the img in, if there's img to push in, using translate. If there's no more image, it stops. Creating a boundary.

I also closed any white space cause by a transform, like rotate, flip or scaling down.

3

My questions

I would like to share this with you if you'd like, but I'm not sure how. It's my first time and I figuered better ask than assume.

I created an image position service. It's a good chunk of code cos it involved quite a lot of math. Hope that's okay. I tried to simplify as much as possible.

I did not create the ability to choose whether to allow image movement or close white space. Both always happen. I think I can add the ability to if you want it.

I understand I fork, clone to local, branch, make changes, commit and request a pull. But do I branch from the current state and remove the previous way of panning the image, or do I branch from say January 13 2022 before the image panning was added?

@Mawi137
Copy link
Owner

Mawi137 commented Dec 17, 2022

Hi
Sounds cool! Your PR should be from the current state of the master branch, else it cannot be merged if there are conflicts.

@loiddy
Copy link
Contributor Author

loiddy commented Jan 4, 2023

Hiya,

I'm close to sending you my PR. But I've encountered something I'm not sure about while tweaking crop().

ImageCroppedEvent sends cropperPosition, imagePosition and offsetImagePosition. When the image has been transformed through CSS, the values sent don't add up to me. But maybe I'm not understanding it properly.

I understand that...

cropperPosition is the position of the cropper in the cropper container - which is the size of the displayed img without any transforms applied to it.

imagePosition is the position of the cropped image in the original image, which includes padding if containWithinAspectRatio is applied.

offsetImagePosition is the same as imagePosition but without the padding if containWithinAspectRatio is applied.

If this is right, shouldn't the transformations applied also be considered to calculate imagePosition and offsetImagePosition? Currently it only uses cropper position and the ratio between sourceImage and loadedImage.

@Mawi137
Copy link
Owner

Mawi137 commented Jan 4, 2023

Hi
Possibly I forgot about those last 2 when the image transformations were added :) So yes, they should in fact be applied there as well.

@loiddy
Copy link
Contributor Author

loiddy commented Jan 5, 2023

I'll look into it. Might take another wee while, still a bit busy with Christmas.

@loiddy
Copy link
Contributor Author

loiddy commented Jan 25, 2023

Hiya! I haven't forgotten about this. While testing I realised I wasn't handling changes in canvas properly. So I'm not looking at that.

@montella1507
Copy link

Still alive? I think it may be solution for
#545

as well

@junaidmehmood67
Copy link

is there any way if i rotate image and ignore the white part, the problem is anytime i rotate the photo it gets the extra white part in it.

https://ibb.co/1Zj9bRm

@loiddy
Copy link
Contributor Author

loiddy commented Aug 1, 2023

@junaidmehmood67 Yes, the code I wrote calculates the smallest possible scale that "covers" the white parts. So the image is enlarged from the center of the cropper to a scale where there is no more white parts. And if I remember correctly, if the image is already scaled at or above that minScale for no white parts, then the image is moved, "closing" the white space. I have some time off for the next weeks and can work on this some more. Would you like to test my work with me?

@montella1507 thank yo for enquiring about my wellbeing. I'm alive and well thank you :) And happy to see that problem was resolved

@Mawi137 I bumped into and fixed issues as I did the transform service. I did it all without many commits cos I confused myself on how to do it along the way, thinking one big one might be the best. I now think better fix the issues, and then add the service and tweak where needed. Probably better to understand what I've done and might be easier to change the code if you don't like something I've done.

Copy link

stale bot commented Dec 15, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants