image-png icon indicating copy to clipboard operation
image-png copied to clipboard

Tracking issue for semver-breaking v0.18

Open Shnatsel opened this issue 1 year ago • 6 comments

We'll want to get these changes done for the semver-breaking v0.18 release:

  • #503
  • #400 (already has a duplicate now: #516 )

Open questions:

  • Do we want to disable broken streaming APNG encoding? #411
  • Do we want to merge #421? It seems the consensus is to go with #493 instead, but we need to commit to one of these options.
  • Are #418 and #253 still relevant? If so, this seems like a good time to fix them.
  • Do we care to fix #521 ?
  • #522

Proactive Breaking changes

  • #590
  • #608

Shnatsel avatar Sep 27 '24 20:09 Shnatsel

Another possible item would be adding BufRead and/or Seek bounds for the decoder. The image crate already requires them, so no further breakage would be required there.

  • Discussion of BufRead: https://github.com/image-rs/image-png/discussions/416#discussioncomment-8088740
  • One usecase for Seek: https://github.com/image-rs/image-png/issues/510, though it could also be helpful for removing the need for PngDecoder::with_limits in the image crate.

fintelia avatar Sep 28 '24 00:09 fintelia

Do we want to merge https://github.com/image-rs/image-png/pull/421? It seems the consensus is to go with https://github.com/image-rs/image-png/pull/493 instead, but we need to commit to one of these options.

I am fine with either approach.

I think that https://github.com/image-rs/image-png/pull/421 results in a slightly cleaner API (no read_... vs next_... discrepancy; can remove Row and InterlacedRow structs). OTOH, https://github.com/image-rs/image-png/pull/493 may still be better overall:

  • Some clients may not care about copying - in these cases its nice if png crate helps with buffer management. (e.g. it was easier to start SkPngRustCodec because of this)
  • Avoiding breaking changes is valuable even if it results on a slightly more verbose/clunkier API (and it's not that bad anyway)

I think that we agree that these changes will be beneficial for performance. OTOH, maybe we can/want to wait to measure the actual impact in Chromium usage (especially since avoiding the extra copy also requires some Chromium-side work). I am totally fine with waiting (the new benchmark results would hopefully come before the end of 2024; nothing is blocked on my side without these PRs - I can always patch them in as long as there is a consensus that one of them will get merged eventually).

anforowicz avatar Sep 28 '24 14:09 anforowicz

Shall we start merging breaking changes?

0.17.0 has been released in 2021.

kornelski avatar Dec 18 '24 02:12 kornelski

We really should get a bug-fix release out to address #546, but after that I think it would be good to decide what we want to include in 0.18 and start merging things

fintelia avatar Dec 18 '24 08:12 fintelia

There have been 0.18 pre-releases, but I can't see progress on https://github.com/image-rs/image-png/milestones Is it outdated or not needed anymore? There still are some open issues referenced here...

reneleonhardt avatar Jun 25 '25 19:06 reneleonhardt

@reneleonhardt Closed issues are not shown by default in that view, so that may be confusing. Also one major open issue with an API break is related to limits, but we're near completion. the other is the interface of the encoder, waiting on an idea that convincingly demonstrates how each of the decoder interfaces can be mirrored into an encoder. Other than those, bug fixes and improvements will happen as minor releases of 0.18.? like usual.

197g avatar Jun 26 '25 14:06 197g