Support Paparazzi#gif with APNGs

[gamepro65]

Handles all the different error configurations that can happen during gif verification. By default the verifier is only used for gif, potentially could be used for all snapshots but wanted to take it one step at a time. General performance testing a base gif during recordPaparazzi averaged ~35ms per frame on an M1 Pro with verifyPaparazzi delta video (which is 3x as wide) taking upward of ~130ms per frame.

Refactored the hard coded failure dir from ImageUtils so we can seed it properly. For testing the delta image generation we opt out of rendering java text (IE. Expected / Actual) because it is different between platforms and would require a non-zero % diff image assertion.

Mismatched frames:

More frames than golden:

Less frames than golden:

Mismatched fps:

[saket]

HYPE!

[saket]

Will this be compatible with composables?

[gamepro65]

Will this be compatible with composables?

Yup! Works as expected out of the box 🙌

[oldergod]

What's blocking this?

[geoff-powell]

This PR looks good to me. Anything blocking this from getting merged? cc @gamepro65

[BrianGardnerAtl]

Looks good. Are you planning on adding the HtmlReportWriter test for the .apng file check?

[jrodbx]

Partial feedback; will give another pass, but wanted to get this out to start the conversation.

[jrodbx]

More feedback, mostly on the record/report mode; will give one last pass for verification.

[jrodbx]

awesome stuff, @gamepro65!

[jrodbx]

before the delta was only drawn on error, but this proposes drawing it on every similarity check. is there any perf impact to take into account here, i.e., does this contribute much to that ~130ms per frame analysis?

[gamepro65]

The delta image is generated while calculating the % error, we previously were just throwing it away when no error.

[jrodbx]

this method is tough to read, given all the implicit receivers. consider rewriting as:

  private fun ApngReader.initializeWriter(): ApngWriter {
    val writer = ApngWriter(deltaFilePath, commonFrameRate, fileSystem)

    val currentFrameNumber = frameNumber - 1
    reset()
    repeat(currentFrameNumber) {
      val nextFrame = readNextFrame() ?: blankFrame
      val (deltaImage) = ImageUtils.compareImages(
        goldenImage = nextFrame,
        image = nextFrame,
        withText = withErrorText
      )
      writer.writeImage(deltaImage)
    }
    readNextFrame()

    return writer
  }

or

  private fun initializeWriter(reader: ApngReader): ApngWriter {
    return ApngWriter(deltaFilePath, commonFrameRate, fileSystem).apply {
      val currentFrameNumber = reader.frameNumber - 1
      reader.reset()
      repeat(currentFrameNumber) {
        val nextFrame = reader.readNextFrame() ?: blankFrame
        val (deltaImage) = ImageUtils.compareImages(
          goldenImage = nextFrame,
          image = nextFrame,
          withText = withErrorText
        )
        writeImage(deltaImage)
      }
      reader.readNextFrame()
    }
  }

to reduce one of the implicit receivers. FWIW, I like the first slightly better.

[gamepro65]

It seems like the hard part is discerning reader from writer. I think this would strike a happy medium

  private fun ApngReader.initializeWriter(): ApngWriter {
    val currentFrameNumber = frameNumber - 1
    reset()

    return ApngWriter(deltaFilePath, commonFrameRate, fileSystem).apply {
      repeat(currentFrameNumber) {
        val nextFrame = readNextFrame() ?: blankFrame
        val (deltaImage) = ImageUtils.compareImages(
          expected = nextFrame,
          actual = nextFrame,
          withText = withErrorText
        )

        writeImage(deltaImage)
      }
      readNextFrame()
    }
  }

[jrodbx]

this is a work of art. kudos!

[jrodbx]

I really like this test