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

Changes for color picker kit #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KevinZhang19870314
Copy link

fix: 1. fix dart warnings; 2. ColorSucker sounds weird, change it to ColorPicker make more sence.

@KevinZhang19870314 KevinZhang19870314 changed the title fix: 1. fix dart warnings; 2. ColorSucker sounds weird, change it to ColorPicker make more sence. Changes for color picker kit Jul 30, 2021
@KevinZhang19870314
Copy link
Author

KevinZhang19870314 commented Jul 30, 2021

feat: add a handler for color picker kit to make it more like magnifier, which will make it more accurate to picker a color.

@KevinZhang19870314
Copy link
Author

@smileShirely feel free to review pr, thanks!

@saan5
Copy link

saan5 commented Jul 31, 2021

access

Copy link

@Vanson Vanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass

Copy link
Collaborator

@smileShirely smileShirely left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KevinZhang19870314 I'm glad to see your PR. But could you consider split the changes to multiple PRs?
And the new feature of color picker seems to be a small problem. The picking point has a incorrect offset.

final Matrix4 newMatrix = Matrix4.identity()
..translate(newX, newY)
..scale(_scale, _scale)
..translate(-newX, -newY);
_matrix = newMatrix;
_searchPixel(dragDetails.globalPosition);
_searchPixel(Offset(newX, newY));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it on multiple devices and the color picking point does not work correctly. Just like the screenshot.
IMG_AAB2C280482E-1

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be known issue in original code, not cause by the PR. @smileShirely

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
Sorry.. I don't think so. The screenshot shows the same pixel has difference RGB values between yours and the original one.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, you can see the ss below, I am using the original code and test it on one plus 8T.

The repro branch

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 this pull request may close these issues.

None yet

4 participants