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

grayscale filter fix #681

Merged
merged 4 commits into from Apr 18, 2019

Conversation

Ni3verma
Copy link
Contributor

@Ni3verma Ni3verma commented Apr 4, 2019

Description

grayscale image is getting created, no error if we press cancel and other enhancement options are also working fine

Fixes #590

Type of change

Just put an x in the [] which are valid.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

  • ./gradlew assembleDebug assembleRelease
  • ./gradlew checkstyle

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@codeclimate
Copy link

codeclimate bot commented Apr 4, 2019

Code Climate has analyzed commit 6bb3ce4 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Contributor

@vyankatesh24 vyankatesh24 left a comment

Choose a reason for hiding this comment

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

@Ni3verma
Still, I am getting error Error on creating application folder while creating grayscale PDF

@Ni3verma
Copy link
Contributor Author

Ni3verma commented Apr 5, 2019

I am not getting any error. Please tell me steps to reproduce the error

@vyankatesh24
Copy link
Contributor

I am not getting any error. Please tell me steps to reproduce the error

Same as in issue #590

@Ni3verma
Copy link
Contributor Author

Ni3verma commented Apr 5, 2019

No error on my device. Are you running it on emulator? Try again after reinstalling the app.

@vyankatesh24
Copy link
Contributor

No error on my device. Are you running it on emulator? Try again after reinstalling the app.

I have already tried reinstalling.

@Ni3verma
Copy link
Contributor Author

Ni3verma commented Apr 5, 2019

I am not getting any error.
@Swati4star @sidhuparas please check if you are also getting the error

@vyankatesh24
Copy link
Contributor

I am not getting any error.
@Swati4star @sidhuparas please check if you are also getting the error

Try reinstalling the app. The error comes when you do grayscale PDF for the first time. Then after it's working already.

Your solution is solving one point in issue

  • Images shouldn't become grayscale when Cancel is pressed.

@Ni3verma
Copy link
Contributor Author

Ni3verma commented Apr 7, 2019

Still no error on my device. Please ask other mentors to test on their device.

@Ni3verma
Copy link
Contributor Author

Ni3verma commented Apr 7, 2019

I just encountered the error...but it was totally random.
I am not able to reproduce it again.
Only thing I did different was that I selected 4 images instead of 2

@sidhuparas
Copy link

@Ni3verma Sorry for the delay, will have a look by evening.

@sidhuparas
Copy link

@Ni3verma Same. Not working for the first time but then working fine onwards. This is getting repeated on every app restart.

@vyankatesh24
Copy link
Contributor

I just encountered the error...but it was totally random.
I am not able to reproduce it again.
Only thing I did different was that I selected 4 images instead of 2

Are you able to find a solution?

@Ni3verma
Copy link
Contributor Author

Ni3verma commented Apr 9, 2019

@vyankatesh24
trying to find the problem.

@Ni3verma
Copy link
Contributor Author

Ni3verma commented Apr 9, 2019

@Ni3verma Same. Not working for the first time but then working fine onwards. This is getting repeated on every app restart.

I was launching the app from recent menu, that's why I was not getting this error. thanks for this info ✌️

@Ni3verma
Copy link
Contributor Author

Ni3verma commented Apr 9, 2019

@vyankatesh24 @sidhuparas
There is some problem with grayscale transformation of picasso.
See getTarget method in imagesToPdfFragment. It's onBitmapLoaded is not getting called for every image.
I need help here.

@sidhuparas
Copy link

@Ni3verma I did some logging and got this in CreatePdf class's doInBackground():
The document has no pages.
This might help a bit. Also the grayscale photos aren't getting removed from Gallery after operation completion.

@Ni3verma
Copy link
Contributor Author

Ni3verma commented Apr 9, 2019

@sidhuparas I know about this log message. It happens because images are not being converted to grayscale.

@Ni3verma
Copy link
Contributor Author

@sidhuparas @vyankatesh24 @Swati4star
I need help with this issue

@vyankatesh24
Copy link
Contributor

@sidhuparas @vyankatesh24 @Swati4star
I need help with this issue

Error is with getTarget() method. in saveCurrentImageInGrayscale

@Ni3verma
Copy link
Contributor Author

@vyankatesh24 @sidhuparas
There is some problem with grayscale transformation of picasso.
See getTarget method in imagesToPdfFragment. It's onBitmapLoaded is not getting called for every image.
I need help here.

@vyankatesh24 I already know about it. guide me how to solve it

@vyankatesh24
Copy link
Contributor

@Ni3verma
What you can do is add Transformed URI to different ArrayList.
Don't remove URIs from mImagesUri in for loop
Outside for loop check, if both are equal in size.
If yes replace mImagesUri ArrayList items
If no reiterate transformation process

@Ni3verma
Copy link
Contributor Author

Please review it again, now problem should be solved. It is a super hard issue !

@vyankatesh24
Copy link
Contributor

I've tested the changes, the issue is solved.
Well Done @Ni3verma 👍

@Swati4star
Copy link
Owner

@vyankatesh24 Please merge this

@vyankatesh24 vyankatesh24 merged commit 8d30ede into Swati4star:master Apr 18, 2019
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.

Error when creating grayscale pdf
4 participants