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

Zoom and rotatión features #11

Open
Velkamhell97 opened this issue Jun 1, 2022 · 20 comments
Open

Zoom and rotatión features #11

Velkamhell97 opened this issue Jun 1, 2022 · 20 comments

Comments

@Velkamhell97
Copy link

This is a awesome package, is the only that allows to custom the UI as you want, please if you could add the zoom and rotate methods to the controller it would be perfect, great work, thanks for your dedication.

@deakjahn
Copy link
Owner

deakjahn commented Jun 1, 2022

Well, I'll think about it. :-)

@monsieurtanuki
Copy link
Contributor

@deakjahn For the record I've implemented a 0-90-180-270 rotation tool based on your code, in openfoodfacts/smooth-app#2872.
If you're interested feel free to find inspiration there or to tell me if you'd like me to code a PR.

@deakjahn
Copy link
Owner

Yes, sure, why not. I couldn't yet take a closer look, is this something that would be easy to incorporate, with retaining existng functionality, without breaking changes?

@monsieurtanuki
Copy link
Contributor

Quickly said, the differences I see:

  • crop_grid.dart: as far as I remember, identical
  • rotated_crop_controller.dart: additional Rotation parameter; the croppedBitmap returns an image (from image/image.dart)
  • rotated_crop_image.dart: now I use a CustomPainter to display the rotated photo (I don't think you used one), and for that I think I changed the image constructor parameter (now it's final ui.Image image), which is probably a breaking change

@deakjahn
Copy link
Owner

deakjahn commented Sep 1, 2022

For the last one I would then suggest a new, named constructor. We can avoid the breaking change that way, besides if somebody doesn't need the rotation at all, they would see no difference at all.

@monsieurtanuki
Copy link
Contributor

Indeed. What's the next step now?

@deakjahn
Copy link
Owner

deakjahn commented Sep 1, 2022

Well, the easiest for me would be if you could submit a PR that does everything. ;-)

@monsieurtanuki
Copy link
Contributor

Makes sense, unfortunately ;)
I'll send a PR within a week.
I'm not sure how integrated the current crop and "my" rotation crop can be. We'll see.

@monsieurtanuki
Copy link
Contributor

@deakjahn Sorry for the delay; I should be able to PR this week.
Beyond the rotation, there's an additional feature: giving enough space to corners so that their related touch area is as big as if in the middle of the image.
Not related: what's the name of the kitten?

tool capture
Screenshot_2023-02-14-19-32-18 Screenshot_2023-02-14-19-34-22

@deakjahn
Copy link
Owner

deakjahn commented Feb 15, 2023

OK, waiting for it.

There was no name. The story is a bit longer than with a regular house cat. I live in the city but in a district with a lot of green areas. Back then, it was about ten years ago, we regularly had a few stray cats living in these gardens, roaming freely (I actually don't know if they were stray or feral in the strictest sense of the word but certainly not wild in any way, just free). I decided to feed one regularly, every day, and we became good friends. This was the mother. And she disappeared one day for a month or so, giving birth to kittens (probably in the cellar of the neighboring house but I'm not sure). She was so friendly, well tempered, calm and trusting that when she first appeared with five kittens in our garden, she simply sat aside and let me, and later other people, including children handle the kittens and play with them without any fear.

I made hundreds of pictures and short videos back then in those weeks, with the intention to collect enough good ones for a printed year calendar on the wall and I actually did, this one being one of the selected 12+1 best pictures at the end. The five kittens then went to different people, we struggled to find new homes for them. This one looked the weakest of the litter, but actually, I received a message back about the strongest one that he (I seem to recall that was a tomcat but I'm not sure any more) didn't live very long later. I have no idea of the fate of the rest.

Anyway, we brought the mother to the vet the next time around, to avoid more kittens, and it was a good thing to do because, as the vet said, there were so many kittens the second time that she would have been unable to give birth and survive the kittening. A friend accepted her during the required few rest days after the operation, and eventually adopted her. In spite of being stray or feral or whatever, she adapted very soon and became a real house cat, literally in a few days' time, probably seeing how much better of a life she had there :-), but as I said, she was of a very calm and well tempered nature to start with. And the surroundings were even more favorable, a detached family house with even more green and free space to roam in the neighborhood. She lived quite a few years there, and although we were never sure of her precise age (the vet said it was much more difficult to tell with cats than with dogs), she probably lived a near normal lifespan.

But I still have the large selection of pictures and use them whenever I have the need for sample images. :-)

@monsieurtanuki
Copy link
Contributor

Thank you @deakjahn for that touching story! Now the kitten will live forever, in github.

If I go back to the curent issue, I'm confronting different problems:

  • adding aspect ratio to rotation + minimum crop size is a bit complex - it's just a question of me being lazy fixing it (so it's just a matter of time)
  • in my own project I need to be immediately aware of the crop changes, like listening to _CropControllerValue - I think it would be interesting in general (not only for rotation) but would need some refactoring and probably (minor) breaking changes. That would mean a version 2.0.0 - do you agree with that?

@monsieurtanuki
Copy link
Contributor

@deakjahn I think I found a lighter solution based on _controller.addListener(() {}), so this will probably not be a breaking change after all.

@deakjahn
Copy link
Owner

@monsieurtanuki If you found a solution, sure, or I would also think about a stream that sends some kind of events and then anybody can subscribe to it if interested.

@PavieOlivier
Copy link

PavieOlivier commented Mar 7, 2023

@monsieurtanuki Any updates by any chance?
Did you push your code on a separate branch?

Edit: nvm, I just went on your profile... I will try checking it out...
But what is the current state?

@monsieurtanuki
Copy link
Contributor

Hi @PavieOlivier!
My latest PR was merged here: the 90 degree rotations are available.
The only thing problematic - according to @deakjahn and I can understand it - is that the crop area gets smaller and smaller when you're rotating with a fixed spect ratio.

@deakjahn
Copy link
Owner

deakjahn commented Mar 7, 2023

I was just waiting a bit. The behavior in the example isn't terribly important in the short run, so if @monsieurtanuki thinks it can be rectified rather easily, I thought we could just wait for it to be done. But if not, I can publish it all right as it is now.

@monsieurtanuki
Copy link
Contributor

To me the quick fix (solution 1) is "if you rotate while using an aspect ratio, after rotation you get the biggest crop area with this aspect ratio". I could code that within a week I guess - but I'm a bit busy currently.

If the fix is (solution 2) "I want the same number of pixels (image pixels? screen pixels?) in the crop areas of all rotations", I'm not sure I would be able to code that quickly, and I would prefer not to.

As an end user I can understand that the current solution (shrinking crop areas) is not good.
Solution 1 is consistent ("always max size") and as I say, "good enough is good enough".

@deakjahn @PavieOlivier Do you agree with solution 1?

@deakjahn
Copy link
Owner

deakjahn commented Mar 7, 2023

Yes, sure. It's unlikely to be a legitimate user operation to turn it round and round, just that we don't offer up an example that comes back with bug reports about the crop disappearing... :-)

@PavieOlivier
Copy link

Yes,
I think solution 1 will be better

@monsieurtanuki
Copy link
Contributor

@deakjahn @PavieOlivier cf #19

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

4 participants