search

mm2 / Little-CMS

A free, open source, CMM engine. It provides fast transforms between ICC profiles.
https://www.littlecms.com
MIT License
549 stars 174 forks source link

Backwards incompatible API-change in 2.16 #429

Closed thomas-weber closed 8 months ago

thomas-weber commented 8 months ago

Hi, version 2.16 remove cmsGetToneCurveParams(). While the change was intentional, this has broken backward compatibility: 03020d0fdd0f8a79f275657ad1ff757fe2cc4192 The function was not documented, but nonetheless part of the public API. I see two possibilities here:

  1. Re-introduce the function (maybe with a deprecation warning).
  2. Bump the SONAME.
mm2 commented 8 months ago

IMHO anything that's undocumented (and accessible) is "use at your own risk." It could go away or change at any time and is not supported. Those functions exist because testing or other reasons but are not intended for normal operation.

Validating this would encourage the use of other undocumented functions. And I dont want this because it means supporting bug reports, adding test cases etc.

DominikDeak commented 8 months ago

That's rather unfortunate. A few years ago I asked if there is a way to access the parametric constants for tone curves. I was suggested to use cmsGetToneCurveParams, but evidently that's no longer available in v2.16.

https://github.com/mm2/Little-CMS/issues/218

Having said that, is using cmsGetToneCurveSegment to query the Params the correct alternative approach?

mm2 commented 8 months ago

That's rather unfortunate.

Why? You were told a function would be added to development sources

I will add this to the development sources, thanks for the idea!

It was on testing from 2.11 to 2.16 where it finally made its way to the public API, with a small change to fix a defect, which was the function didn't work for multi-segment. This is the normal developing cycle.

Please remember this is open source under MIT license, basically you have the code for free but with no support. That means you need to do a check when upgrading and keep track of this kind of things. If this does not fit your needs, I would recommend to go to any commercial library.

mm2 commented 8 months ago

Closing it as not planned

DominikDeak commented 8 months ago

Please don't take my above comment the wrong way. I was merely saying it's an unfortunate situation for us that relied on those undocumented features. And I agree, the responsibility (and the consequences) for using private APIs rests on our shoulders. I did not intend lay blame on you or anyone for these breaking changes, other than myself.

You were told a function would be added to development sources

It was on testing from 2.11 to 2.16 where it finally made its way to the public API

I'm sorry, I must have missed this change in the public API. I skimmed through the ChangeLog, but I could not find notes about new features that queries Tone Curve parametric constants. I'm assuming cmsGetToneCurveSegment is now the correct way to get this data?

Please remember this is open source under MIT license, basically you have the code for free but with no support. That means you need to do a check when upgrading and keep track of this kind of things. If this does not fit your needs, I would recommend to go to any commercial library.

Yes, I am fully aware. And I am actually very grateful for the effort you put into maintaining this project. Thank you!