Re: [Lcms-user] [Release candidate] Little CMS 2.14 is ready to be released

[<mar...@li...>]

Hi

> If so, a random thought...  On x64 (with the Microsoft C++ compiler at least), just an (int) cast from a 32 bit float to 32 bit int, which compiles down to one x86 machine instruction - cvttss2si - will truncate to the next integer value in the direction of 0.  No need to call a floor function at all.

Thanks. Nowadays  I tend to write code for clarity and let the compiler do the optimizations. In this case, it is hardly compiler dependent. On ARM for example, clang 11 generates fcvtms versus  fcvtzs on other compiler things may change. For most compilers floorf is an intrinsic. Since the algorithm is described using floor, I use floor instead of an int cast. In fact, the mantissa trick I was using is no longer faster that the floorf function. After all the optimizer rearranges code so it depends on how well the whole thing parallelizes. Floorf uses xxmm instructions and those parallelize very well. 

Regards

Marti Maria
The LittleCMS Project
https://www.littlecms.com

-----Original Message-----
From: Noel Carboni <NCa...@Pr...> 
Sent: Friday, October 21, 2022 6:18 AM
To: mar...@li...
Cc: lcm...@li...
Subject: RE: [Lcms-user] [Release candidate] Little CMS 2.14 is ready to be released

Hi Marti,

> Found the bug and fixed it in
> https://github.com/mm2/Little-CMS/commit/aa7f240da8dc27a0bb082462889cc
> a9026bf2b27

Are all your px, py, pz values always non-negative at the point of the above changes to fast_float_tethra.c?  

If so, a random thought...  On x64 (with the Microsoft C++ compiler at least), just an (int) cast from a 32 bit float to 32 bit int, which compiles down to one x86 machine instruction - cvttss2si - will truncate to the next integer value in the direction of 0.  No need to call a floor function at all.

If those values can be negative...  Never mind.  😊

-Noel Carboni
 ProDigital Software

-----Original Message-----
From: mar...@li... <mar...@li...>
Sent: Thu, October 20, 2022 1:24 PM
To: 'L. E. Segovia' <am...@am...>
Cc: lcm...@li...
Subject: Re: [Lcms-user] [Release candidate] Little CMS 2.14 is ready to be released

Hello,

Found the bug and fixed it in
https://github.com/mm2/Little-CMS/commit/aa7f240da8dc27a0bb082462889cca9026bf2b27

It is an AMAZING bug, triggered by an issue of precision loss. 

For those that enjoy this kind of things I will briefly describe what was happening.

I used a tweaked floor() for speed. floor() function is supposed to return the largest integer value that is less than or equal to a number. My tweaked function was using mantissa tricks to run as fast as possible. It worked for all but a small number of combinations. 
Well, you hit one of those bad combinations. A number like 47.9993 (I think, I may be wrong on recalling the exact number). Quick floor of this particular number returned 48 when it really was 47. At the end its only 0.0007 difference so may seem harmless...
BUT this was used for interpolation so a subtraction was involved. if we try to compute the node with that faulty floor and then subtract values to get the rest, we end in a negative number. And you know, negative number mixes bad with unsigned, so at the end this was creating a nasty out-of-bounds condition.  Funny enough, very few numbers are capable of that. You were lucky enough to discover the bug because you hit a bad number with white with those profiles and flags otherwise this could go unnoticed for years.

Many thanks for reporting. You earned the award of "bug of the year" because its rarity.

Best regards
Marti Maria
The LittleCMS Project
https://www.littlecms.com

-----Original Message-----
From: L. E. Segovia <am...@am...>
Sent: Thursday, October 20, 2022 3:46 AM
To: mar...@li...
Cc: lcm...@li...
Subject: Re: [Lcms-user] [Release candidate] Little CMS 2.14 is ready to be released

Hi,

The Krita code for the LCMS transform initialization is a nightmare... 
apart from a 10 y-o typo in the format definition (not declaring FLOAT_SH where we should), I've managed to consistently reproduce the bug.

I've adjusted two lines below:

> #include <lcms2.h>
> #include <lcms2_fast_float.h>
> 
> int main(void)
> {
>     cmsHPROFILE hInput, hOutput;
>     cmsHTRANSFORM xform;
> 
>     cmsPlugin(cmsFastFloatExtensions());
> 
>     hInput = cmsOpenProfileFromFile("profile.icc", "r");
>     hOutput = cmsOpenProfileFromFile("Cintiq 13 #2 2022-07-29 D6500
> 2.2 S XYZLUT+MTX sRGB-HQ.icm", "r");
> 
>     xform = cmsCreateTransform(hInput, TYPE_RGBA_FLT, hOutput, 
> TYPE_RGBA_FLT, INTENT_PERCEPTUAL, cmsFLAGS_BLACKPOINTCOMPENSATION | 
> cmsFLAGS_HIGHRESPRECALC);
> 
>     float ones[4] = {0.999999821f, 1.0f, 5.05994073e-08f, 1.0f };
>     float result[4] = { 0 };
> 
>     cmsDoTransform(xform, ones, result, 1);
> 
>     cmsCloseProfile(hInput); cmsCloseProfile(hOutput);
>     cmsDeleteTransform(xform);
>     return 1;
> }

We at Krita use RGBA, and the given conversion flags: 
cmsFLAGS_BLACKPOINTCOMPENSATION and cmsFLAGS_HIGHRESPRECALC.

The pixel values were extracted straight from the exception stacktrace, and I've verified them separately.

Best,

amyspark

On 19/10/2022 09:08, mar...@li... wrote:
> 
> Hi,
> 
> I've tried the following code with the plug-in and your profiles. It works fine.  Please let me know which differences are in the Krita call because I cannot reproduce the error.
> 
> int checkKrita(void)
> {
>      cmsHPROFILE hInput, hOutput;
>      cmsHTRANSFORM xform;
> 
>      cmsPlugin(cmsFastFloatExtensions());
> 
>      hInput = cmsOpenProfileFromFile("profile.icc", "r");
>      hOutput = cmsOpenProfileFromFile("Cintiq 13 #2 2022-07-29 D6500
> 2.2 S XYZLUT+MTX sRGB-HQ.icm", "r");
> 
>      xform = cmsCreateTransform(hInput, TYPE_RGB_FLT, hOutput, 
> TYPE_RGB_FLT, INTENT_PERCEPTUAL, 0);
> 
>      float ones[3] = { 1.0f, 1.0f, 1.0f };
>      float result[3] = { 0 };
> 
>      cmsDoTransform(xform, ones, result, 1);
> 
>      cmsCloseProfile(hInput); cmsCloseProfile(hOutput);
>      cmsDeleteTransform(xform);
>      return 1;
> }
> 
> BTW, are you linking the plug-in as a DLL?
> 
> Regards
> 
> Marti Maria
> The LittleCMS Project
> https://www.littlecms.com
> 
> -----Original Message-----
> From: L. E. Segovia <am...@am...>
> Sent: Wednesday, October 19, 2022 2:38 AM
> To: Marti Maria <mar...@li...>
> Cc: lcm...@li...
> Subject: Re: [Lcms-user] [Release candidate] Little CMS 2.14 is ready 
> to be released
> 
> Hey Marti,
> 
> I've been able to reproduce the bug in both 2.13.1 and the recent release candidate, both point to this stacktrace (snipped for brevity) inside the tetrahedral RGB interpolation module:
> 
> lcms2_fast_float.dll!FloatCLUTEval(_cmstransform_struct * CMMcargo, 
> const void * Input, void * Output, unsigned int PixelsPerLine, 
> unsigned int LineCount, const cmsStride * Stride) Line 199 
> (e:\krita-win\patches\Little-CMS\src\plugins\fast_float\src\fast_float
> _tethra.c:199) lcms2.dll!cmsDoTransform(void * Transform, const void * 
> InputBuffer, void * OutputBuffer, unsigned int Size) Line 207
> (e:\krita-win\patches\Little-CMS\src\src\cmsxform.c:207)
> 
> The bug can be reproduced with 1.0 in any of the input's channels, and the source and display profiles provided in the bug report.
> 
> Is there anything else I can do to help you reproduce it?
> 
> Best,
> 
> amyspark
> 
> On 16/10/2022 16:40, Marti Maria wrote:
>> Hello,
>>
>> Could you try with 2.14rc1? there are some fixes on the plugin picking optimisations where it should not. Chances are that this is solved.
>>
>> Thanks
>> Marti
>>
>>
>>> On 16 Oct 2022, at 19:56, L. E. Segovia via Lcms-user <lcm...@li...> wrote:
>>>
>>> Hey Marti,
>>>
>>> We've gotten a bug report at Krita regarding the fast float plugin:
>>>
>>> https://bugs.kde.org/show_bug.cgi?id=460512
>>>
>>> Haven't yet had the time to symbolicate it, so sending this to keep you aware of it.
>>>
>>> Best,
>>>
>>> amyspark
>>>
>>> On 15/10/2022 15:38, Marti Maria wrote:
>>>> I am glad to announce the imminent release of lcms2-2.14 The first 
>>>> release candidate is available here:
>>>> https://github.com/mm2/Little-CMS/releases/tag/lcms2.14rc1
>>>> Changes
>>>> 	• lcms2 now fully implements ICC specification 4.4
>>>> 	• New multi-threaded plug-in
>>>> 	• several fixes to keep fuzzers happy
>>>> 	• Remove check on DLL when CMS_NO_REGISTER_KEYWORD is used
>>>> 	• Added more validation against broken profiles
>>>> 	• Add more help to several tools
>>>> ICC Version 4.4 and the multithreaded plug-in are important milestones. Fixes because fuzzers have been many, none of them harmful in terms of exploits.
>>>> Feel free to test it on all your products, tentative release date for final lcms2-2.14 is end of Oct-2022 Please contact me on any issue either on info { at } littecms.com or using this list.
>>>> Best regards
>>>> Marti Maria
>>>> The LittleCMS Project
>>>> https://www.littlecms.com
>>>> _______________________________________________
>>>> Lcms-user mailing list
>>>> Lcm...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/lcms-user
>>>
>>> --
>>> amyspark 🌸 https://www.amyspark.me
>>>
>>>
>>> _______________________________________________
>>> Lcms-user mailing list
>>> Lcm...@li...
>>> https://lists.sourceforge.net/lists/listinfo/lcms-user
>>
> 
> --
> amyspark 🌸 https://www.amyspark.me
> 

--
amyspark 🌸 https://www.amyspark.me



_______________________________________________
Lcms-user mailing list
Lcm...@li...
https://lists.sourceforge.net/lists/listinfo/lcms-user