Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a maxDifferentPixelsRate parameter to matchesReferenceImage to allow a configurable tolerance rate for pixel differences during reference image comparisons, and updates relevant tests. Feedback suggests returning null early when countDifferentPixels is 0 to prevent potential division-by-zero or NaN issues when calculating the pixel difference rate.
andywolff
marked this pull request as draft
October 29, 2026 22:33
andywolff
force-pushed
the
matches-reference-image-tolerance
branch
from
October 30, 2026 21:47
88abd2d to
97978e1
Compare
Contributor
Author
|
Closing this draft, I found the true root cause of this rendering difference and we can address it without adding a pixel diff tolerance |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #193689
Pre-requisite for #192937
This PR adds an optional
maxDifferentPixelsRatetolerance parameter tomatchesReferenceImageinpackage:flutter_testand configures a small tolerance for Impeller intransform_test.dart.We did the same thing for
engine/src/flutter/testing/dart/goldens.dart'sfuzzyCompareImagesin #193937. Can't share because that's in the engine code, which can't depend on framework packages, andmatchesReferenceImageis inpackage:flutter_test.The test
3D transform renders the same with or without needsCompositingwas originally added in #61779 to prevent visual jumping regressions (#41654) where layer integer coordinate snapping caused 3D transforms to wiggle whenneedsCompositingchanged. Across a loop of angles, it compares a raster snapshot of the transform rendered withneedsCompositing: trueagainst the same transform rendered withneedsCompositing: falseusingmatchesReferenceImage.Under Skia, both composited and non-composited paths render identically with 0 differing pixels through software rasterization.
Under Impeller, they actually don't render identically because they take different paths:
with compositing: the child is drawn into an offscreen texture and composited as a 3D texture quad sampled via bilinear filtering.
Without composition: the canvas renders vector geometry directly under the 3D perspective matrix using MSAA edge coverage.
Because of this difference, there are antialiasing differences along angled polygon boundaries (consistently ~241 to 247 pixels out of 480,000, or ~0.05% of pixels). It's not a full pixel shift like the issue for which this test was added.
But why do these differences only appear with #192937? Because the host buffer wasn't being reset between snapshots, it actually wasn't re-rendering(?).
I previously attempted to mitigate this in #193936 by reducing loop density (increasing the angle step from
0.01to0.05), because I thought it was a problem happening towards the end of the loop after too many iterations. But this didn't help. Now that we have a proper diff tolerance, this PR reverts that angle step back to0.01, restoring the test to its original angular sampling density.Pre-launch Checklist
///).