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

Added image difference #18

Merged
merged 5 commits into from
Jan 13, 2017
Merged

Added image difference #18

merged 5 commits into from
Jan 13, 2017

Conversation

PeterD23
Copy link
Contributor

Overloaded existing imagesAreEquals method to include an output file and pixel manipulation to mark areas of deviation when comparing images.

@glibas glibas self-requested a review January 11, 2017 11:48
@glibas
Copy link
Member

glibas commented Jan 11, 2017

Hi @PeterD23 ,

Thank you for your contribution.

Could you please update commit b1851f1 to include import related changes only, but not other 14 files.

Thanks,
Glib

@PeterD23
Copy link
Contributor Author

My bad, I'm still new to Gradle and the Git CLI. I've removed the unnecessary compiled files from the commit.

@glibas glibas assigned glibas and unassigned PeterD23 Jan 11, 2017
//System.out.println("%"); // Format don't like that syntax!

// Write the image as png, with the filename based on the path provided
FileUtil.writeImage(output,imgExtension,new File(pathFileName));
Copy link
Member

Choose a reason for hiding this comment

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

Add condition to only write image if images are not equal. No need to save additional image if they are same.

Copy link
Member

@glibas glibas left a comment

Choose a reason for hiding this comment

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

Hi @PeterD23 ,

Thank you. Could you please consider mentioned comments.

int colourSpaceBytes = 3; // RGB is 24 bit, or 3 bytes
double totalPixels = width1 * height1 * colourSpaceBytes;
double pixelError = diff / totalPixels / 255.0;
//System.out.format("Pixel error: "+"%.5f",pixelError*100);
Copy link
Member

Choose a reason for hiding this comment

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

Remove commented sysout

* @param deviation The upper limit of the pixel deviation for the test
* @return If the test passes
*/
public static boolean imagesAreEquals(BufferedImage image1, BufferedImage image2, String pathFileName, String imgExtension, double deviation) {
Copy link
Member

Choose a reason for hiding this comment

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

No need to pass imgExtension as a parameter. We are only using png, since any other format will cause conversion discrepancies.

* @param deviation The upper limit of the pixel deviation for the test
* @return If the test passes
*/
public static boolean imagesAreEquals(BufferedImage image1, BufferedImage image2, String pathFileName, String imgExtension, double deviation) {
Copy link
Member

@glibas glibas Jan 12, 2017

Choose a reason for hiding this comment

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

Rename method to imagesAreEqualsWithDiff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will rename it, but I figured that overloading the method would be better since it performs the same function but with added functionality.

@glibas glibas assigned PeterD23 and unassigned glibas Jan 12, 2017
@glibas
Copy link
Member

glibas commented Jan 13, 2017

Also tried running for img1,img2 (attached) and got the output.png. Shouldn't we just highlight new element position and leave original untouched?

img2

img1

output

@PeterD23
Copy link
Contributor Author

PeterD23 commented Jan 13, 2017

For the final comment about showing only the moved position, I am unable to figure out how that can be done, since there is no way I can think of to ignore the original positions.

@glibas
Copy link
Member

glibas commented Jan 13, 2017

Hi @PeterD23 ,

Thanks for update. Yes you are right about skipping formatting the original position. Lets leave it as it is for now.

@glibas glibas merged commit c0f6617 into assertthat:master Jan 13, 2017
@PeterD23 PeterD23 deleted the imageDiff branch January 13, 2017 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants