phetsims / aqua

Automatic QUality Assurance
MIT License
2 stars 4 forks source link

Multi snapshot comparison tool improvements #205

Closed zepumph closed 8 months ago

zepumph commented 8 months ago

I have been using this a bunch over in https://github.com/phetsims/axon/issues/447, and I've made many changes to the tool as I went. I think it is time to capture them in a spot that is specific to multi snapshot, and not my testing needs for the tool

zepumph commented 8 months ago
zepumph commented 8 months ago

After a discussion with @jonathanolson, we found some potential/for-sure bugs in how snapshots were being calculated created failure false positives. Commits coming soon, but here are the items:

zepumph commented 8 months ago

Patch for dealing with single pixel differences in handling (this needs to still be propagated up to calculated IF the row even has a difference, since the hashing doesn't account for these new tolerances.

```diff Subject: [PATCH] add testPhetio options --- Index: js/MultiSnapshotComparison.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/MultiSnapshotComparison.ts b/js/MultiSnapshotComparison.ts --- a/js/MultiSnapshotComparison.ts (revision 35134694c3ce6ee24b32bf1f1ccd52d2004a5659) +++ b/js/MultiSnapshotComparison.ts (date 1710868981846) @@ -161,12 +161,15 @@ const imageA = await loadImage( urlA ); const imageB = await loadImage( urlB ); - const threshold = 0; + const threshold = 0; // TODO: 0.0001? const a = contextToData( imageToContext( imageA, width, height ), width, height ); const b = contextToData( imageToContext( imageB, width, height ), width, height ); let largestDifference = 0; let totalDifference = 0; + let numberOfDifferences = 0; + const numberOfColorsThreshold = 10 * 4; // 10 pixels can be different (this is horrible). + const colorDiffData = document.createElement( 'canvas' ).getContext( '2d' ).createImageData( a.width, a.height ); const alphaDiffData = document.createElement( 'canvas' ).getContext( '2d' ).createImageData( a.width, a.height ); for ( let i = 0; i < a.data.length; i++ ) { @@ -181,6 +184,10 @@ else { colorDiffData.data[ i ] = diff; } + + if ( diff ) { + numberOfDifferences++; + } const alphaIndex = ( i - ( i % 4 ) + 3 ); // grab the associated alpha channel and multiply it times the diff const alphaMultipliedDiff = ( i % 4 === 3 ) ? diff : diff * ( a.data[ alphaIndex ] / 255 ) * ( b.data[ alphaIndex ] / 255 ); @@ -196,7 +203,10 @@ const averageDifference = totalDifference / ( 4 * a.width * a.height ); - if ( averageDifference > threshold ) { + if ( numberOfDifferences > 0 ) { + console.log( 'differences!!!', numberOfDifferences ); + } + if ( numberOfDifferences > numberOfColorsThreshold ) { return { a: dataToCanvas( a, width, height ), b: dataToCanvas( b, width, height ),
zepumph commented 8 months ago

Alright. Lots of good work here today, I am going to kick off a test of two copies of main with this patch to see what the scope of tiny differences are across the project. Otherwise, I believe we are almost done with this issue.

Testing this URL: http://localhost:8080/aqua/html/multi-snapshot-comparison.html?ea&urls=http://localhost,http://localhost&simQueryParameters=ea&numFrames=100&testPhetio=false

```diff Subject: [PATCH] image comparison better thresholds --- Index: js/image-compare.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/image-compare.js b/js/image-compare.js --- a/js/image-compare.js (revision 88a662710d069a2482db923352e4dc9c9448b566) +++ b/js/image-compare.js (date 1710891080360) @@ -48,6 +48,9 @@ let largestDifference = 0; let totalDifference = 0; + let numberOfDifferences = 0; + // const numberOfColorsThreshold = 10 * 4; // 10 pixels can be different (this is horrible). + const colorDiffData = document.createElement( 'canvas' ).getContext( '2d' ).createImageData( a.width, a.height ); const alphaDiffData = document.createElement( 'canvas' ).getContext( '2d' ).createImageData( a.width, a.height ); for ( let i = 0; i < a.data.length; i++ ) { @@ -62,6 +65,10 @@ else { colorDiffData.data[ i ] = diff; } + if ( diff ) { + numberOfDifferences++; + } + const alphaIndex = ( i - ( i % 4 ) + 3 ); // grab the associated alpha channel and multiply it times the diff const alphaMultipliedDiff = ( i % 4 === 3 ) ? diff : diff * ( a.data[ alphaIndex ] / 255 ) * ( b.data[ alphaIndex ] / 255 ); @@ -77,6 +84,13 @@ const averageDifference = totalDifference / ( 4 * a.width * a.height ); + if ( averageDifference < 0.0001 ) { + console.log( 'averageDifference was <.0001:', averageDifference ); + } + if ( numberOfDifferences > 0 ) { + console.log( 'numberOfDifferences:', numberOfDifferences ); + } + if ( averageDifference > threshold ) { const container = document.createElement( 'div' ); const comparisonData = {
zepumph commented 8 months ago

Today I was able to test out what sims have a very small pixel issue. Here are my notes:

To continue testing: balancing-act,beers-law-lab,center-and-variability,charges-and-fields,circuit-construction-kit-ac,circuit-construction-kit-dc,diffusion,energy-skate-park,equality-explorer,equality-explorer-basics,forces-and-motion-basics,gas-properties,interaction-dashboard,natural-selection,neuron,quadrilateral,wave-interference,waves-intro

Here is the quantity of problem for these sims. Most only have a couple of pixels off (very very small differences)

``` balancing act numberOfDifferences: 6 beers law ??? centeranvar averageDifference:0.0000762939453125 numberOfDifferences: 4 CAF averageDiff: 7.8 number: 39907 numberOfDifferences: 42172 image-compare.js:91:15 numberOfDifferences: 76335 CCK number:2 average: 0.000010172526041666666 number:3 average: 0.000020345052083333332 number: 3 average: 0.000020345052083333332 number: 9 average: 0.0003153483072916667 diffusion number: 700 average: :0.2875213623046875 number:1735 average: 0.6362101236979166 ESP number: 24 average: 0.008595784505208334 EExplorere number: 3 average: :0.0001068115234375 number: 6 average: 0.00017801920572916666 FAMB numbrt:1393 average: 0.20409647623697916 interactiondash number:15 average: 0.0012054443359375 natural sele number:7732 average: 2.2993265787760415 neuron number: 18413 average: quad average: :0.0001220703125 number:3 wave-interfer average: 0.00006103515625 number: 3 waves-intro average: 0.0000457763671875 number: 3 ```

Patch:

```diff Subject: [PATCH] toggle images when selecting a row, https://github.com/phetsims/aqua/issues/205 --- Index: js/image-compare.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/image-compare.js b/js/image-compare.js --- a/js/image-compare.js (revision a238eafedc63234f55ce5ea53db3478c8dca4bfc) +++ b/js/image-compare.js (date 1710955248366) @@ -48,6 +48,9 @@ let largestDifference = 0; let totalDifference = 0; + let numberOfDifferences = 0; + // const numberOfColorsThreshold = 10 * 4; // 10 pixels can be different (this is horrible). + const colorDiffData = document.createElement( 'canvas' ).getContext( '2d' ).createImageData( a.width, a.height ); const alphaDiffData = document.createElement( 'canvas' ).getContext( '2d' ).createImageData( a.width, a.height ); for ( let i = 0; i < a.data.length; i++ ) { @@ -62,6 +65,10 @@ else { colorDiffData.data[ i ] = diff; } + if ( diff ) { + numberOfDifferences++; + } + const alphaIndex = ( i - ( i % 4 ) + 3 ); // grab the associated alpha channel and multiply it times the diff const alphaMultipliedDiff = ( i % 4 === 3 ) ? diff : diff * ( a.data[ alphaIndex ] / 255 ) * ( b.data[ alphaIndex ] / 255 ); @@ -77,6 +84,13 @@ const averageDifference = totalDifference / ( 4 * a.width * a.height ); + if ( averageDifference > 0 ) { + console.log( 'averageDifference:', averageDifference ); + } + if ( numberOfDifferences > 0 ) { + console.log( 'numberOfDifferences:', numberOfDifferences ); + } + if ( averageDifference > threshold ) { const container = document.createElement( 'div' ); const comparisonData = {
zepumph commented 8 months ago

Alright. I went over comparison thresholds and extra changes with @jonathanolson this morning, and things are looking very good. All of the above linked sims are now passing when comparing to the same checkout. Closing

phet-dev commented 8 months ago

Reopening because there is a TODO marked for this issue.