-
Notifications
You must be signed in to change notification settings - Fork 85
Generate diff images for web snapshots #2803
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
Conversation
9165ead
to
95de1fc
Compare
769e0f1
to
edae224
Compare
@@ -69,6 +72,7 @@ public class DomSnapshotter @PublishedApi internal constructor( | |||
this.canvasWidth = this.width | |||
this.canvasHeight = this.height | |||
this.pixelRatio = frame.pixelRatio | |||
this.backgroundColor = "transparent" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redwood-layout-dom ci was failing because the default background color is white instead of transparent. Here I'm explicitly setting the background color to transparent so the snapshot background color would be consistent.
from https://github.com/cashapp/redwood/actions/runs/17656504943/artifacts/3990317185
edae224
to
dd2f2fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So cool!
ctx.putImageData(deltaData, maxWidth.toDouble(), 0.0) | ||
|
||
// Convert canvas to blob | ||
val deltaBlob = suspendCancellableCoroutine<Blob> { continuation -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t see why this call is necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to convert the delta image to blob so we can write to file.
val total = maxHeight.toLong() * maxWidth.toLong() * 3L * 256L | ||
var percentDifference = (delta * 100 / total.toDouble()).toFloat() | ||
|
||
// Fallback to pixel difference if color delta is 0 but pixels are different |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case is for when it’s only different on the alpha channel?
|
||
// Fallback to pixel difference if color delta is 0 but pixels are different | ||
if (differentPixels > 0 && percentDifference == 0f) { | ||
percentDifference = (differentPixels * 100 / (maxWidth * maxHeight).toDouble()).toFloat() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should percentDifference be the max of these two measurements?
val percentDifferenceRgb = ...
val percentDifferenceAlpha = ...
val percentDifference = maxOf(percentDifferenceRgb, percentDifferenceAlpha)
?
As is if the alpha difference is 100% but the pixel difference is 0.1%, we’ll report 0.1%.
(That gets to be a practical problem when we support stuff like fuzzy-matching and accept a tolerance of .1%)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you meant take percentDifferenceRgb if it's not 0 else percentDifferenceAlpha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love it.
In a follow-up would you mind extracting this new code into a new class like ImageDiffer
or something, and writing a couple test cases for it directly? I know testing tests is a bit yak-shaving, but in this case the code is subtle enough that I think it’s warranted. Plus I want to extract this code into a standalone library for everyone to use!
In particular I’d like test cases to cover things like
You can probably use code like this to create sample images, but altering sizes or whatever to create diffs.
|
No description provided.