Implement image diff plugin #96
Conversation
|
Easy and customizable dashboards for your build system. Learn more about Stoat ↗︎ Static Hosting
Image Diff
Job Runtime |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
This is awesome!
A few concerns:
- For now, if the user wants to pull an image from an external location, they should do so in a separate step in their GH workflow (we shouldn't do it as part of the plugin). I think this is actually a crucial pattern for us in order to limit complexity for private repos.
- The baseline should default to default-branch output for this plugin
- We need to be able to express that the image didn't originally exist (especially for default branch)
- For the first release of this, it'd be great to have some error handling around non-matching inputs so we just drop the diff instead of fail
I'm also curious what diffs are most important for users. Is it default/baseline to the latest commit? Is it default/baseline to each commit (which implies the need for a separate UI for the plugin). Is pixel diffing enough or do we need swipes/onion skins/resemblejs. I don't think any of these should block the PR but it's worth discussing on our upcoming customer calls. I'm not sure how they think about which data they want to compare. I'm also curious about their tolerance for checking in images into the repo or how many images they actually want to compare on a regular basis. If it's 100s or 1000s for PR, or only ones with changes we may need to make significant changes.
| const diffPath = `${currentDirectory}/${uuid}-diff.png`; | ||
| core.info(`[${taskId}] Creating image diff to ${diffPath}...`); | ||
| const baselineImage = PNG.sync.read(fs.readFileSync(baselinePath)); | ||
| const currentImage = PNG.sync.read(fs.readFileSync(taskConfig.path)); |
There was a problem hiding this comment.
Can this use https://www.npmjs.com/package/jimp or similar to convert to PNG first so we can support jpeg/png/bmp/tiff/gif out of the box?
There was a problem hiding this comment.
Wow, this is super nice. Also this library can resize the image so that we can compare images with different sizes.
There was a problem hiding this comment.
Done. A downside of introducing jimp though is that this library is huge, 3MB, adding 88K lines of code to dist/index.js.
| const currentImage = PNG.sync.read(fs.readFileSync(taskConfig.path)); | ||
| const { width, height } = baselineImage; | ||
| const diffImage = new PNG({ width, height }); | ||
| pixelmatch(baselineImage.data, currentImage.data, diffImage.data, width, height, { threshold: 0.1 }); |
There was a problem hiding this comment.
Not blocking this PR at all, but ideally we should have a link to a static site that takes the user to a page that lets them do similar things to the GitHub image comparison functionality except for non-checked-in images.
Agree. The current implementation downloads a static image mainly for demo and testing purposes.
Yes. This will be implemented in a follow-up PR. It's easier this way because we will have some partial config from the default branch for testing.
I think it's also fine if the image already exists. For example, it can be a file checked into the codebase. It's not the main use case, but it does not hurt to have this extra feature, and some users may like it.
Will do.
Yes. All these depend on customer needs. |
For this we could probably use a gray placeholder image or something or even just text saying it doesn't exist. I just want to make sure it doesn't fail on this (probably common) case. |
Summary
The image diff plugin can compare an image against a baseline one. The image can be static or generated during the build. This plugin will be most useful for machine learning CI.
The logo image is modified for testing purpose only. The change should be reverted before merging.
Limitation
pixelmatchcan only compare images with exact same size.TODOs
Support local or remote URL for bothpathandbaselinefiles.