Skip to content

Commit

Permalink
Fix for optimisation error on grayscale
Browse files Browse the repository at this point in the history 
Thanks to Aaron Boxer for reporting this issue
  • Loading branch information
@mm2
mm2 committed Jan 30, 2022
1 parent ac4fe46 commit fdbfb76
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/cmsintrp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,10 @@ void Eval1Input(CMSREGISTER const cmsUInt16Number Input[],
// if last value...
if (Input[0] == 0xffff || p16->Domain[0] == 0) {

cmsUInt16Number y0 = LutTable[p16->Domain[0]];

cmsUInt32Number y0 = p16->Domain[0] * p16->opta[0];
for (OutChan = 0; OutChan < p16->nOutputs; OutChan++) {
Output[OutChan] = y0;
Output[OutChan] = LutTable[y0 + OutChan];
}
}
else
Expand Down Expand Up @@ -324,10 +324,10 @@ void Eval1InputFloat(const cmsFloat32Number Value[],
// if last value...
if (val2 == 1.0 || p->Domain[0] == 0) {

y0 = LutTable[p->Domain[0]];
cmsUInt32Number start = p->Domain[0] * p->opta[0];

for (OutChan = 0; OutChan < p->nOutputs; OutChan++) {
Output[OutChan] = y0;
Output[OutChan] = LutTable[start + OutChan];
}
}
else
Expand Down

9 comments on commit fdbfb76

@albert-astals-cid-kdab
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mm2 this seems to be the cause of a relatively important regression (see https://bugs.archlinux.org/task/73580 ) are you planning a 2.13.1 release?

@jonasmalacofilho
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small note: the bug this patch fixes seems to be cause of the issue.

(Also, I'm the reporter of that issue, in case there's any other useful information I can provide).

@albert-astals-cid-kdab
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, sorry terrible wording on my side, this fixes the bug, not cause it

@mm2
Copy link
Owner Author

@mm2 mm2 commented on fdbfb76 Feb 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonasmalacofilho @albert-astals-cid-kdab
Hi, thanks for reporting. I have checked the code base and this issue has been there since 2.10, (June-2020), so it is not so critical at all. I will try to reschedule 2.14 to be in 4 months or so, but doing a release now would introduce a lot of noise for something that has passed unnoticed 2 years. Thanks again.

@jonasmalacofilho
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mm2, I see your point. However, it was specifically the update from 2.12 to 2.13 that triggered the symptoms with evince and cups + Epson ESC/P-R. Maybe something else in 2.10–2.12 was masking the problem?

@mm2
Copy link
Owner Author

@mm2 mm2 commented on fdbfb76 Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue can be easely reproduced by using the tificc utility, I used:

tificc -c1 -i"*gray22" input.tif output.tif

Being "input.tif" a TIFF file holding a monochrome image. -c1 means use the default optimisation. Pixels with 255 got wrong color.
It is certainly possible the bug was hidden by others. Or maybe the test image didn't have 255 at all. This is the only value affected.

A quick workaround is to use cmsFLAGS_NOOPTIMIZE when input is grayscale. Otherwise you can also apply the patch.
For me is very difficult to generate a new revision. The building scripts and banners does not use the revision field in major.minor.revision so generating a new release now would be a real mess. I would like also to add test cases in testbed covering this issue and check for different profile types.

@jonasmalacofilho
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed explanation.

Or maybe the test image didn't have 255 at all.

I'm not familiar with how PostScript and PDF store the paper/background color. But the same files worked on 2.12 but and got a gray background on 2.13 (one of them is on linked Arch bug).

A quick workaround is to use cmsFLAGS_NOOPTIMIZE when input is grayscale. Otherwise you can also apply the patch.

Yes, I've manually patched my system, and Arch Linux now has a patched package as well. Patching Little CMS was more convenient for me than going over individual applications that were showing problems, but it's good to know of an alternative workaround.

For me is very difficult to generate a new revision. The building scripts and banners does not use the revision field in major.minor.revision so generating a new release now would be a real mess. I would like also to add test cases in testbed covering this issue and check for different profile types.

I completely understand : )

Thanks again!

@Arusekk
Copy link

@Arusekk Arusekk commented on fdbfb76 Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref. a bug report from Gentoo: https://bugs.gentoo.org/832520

@mm2
Copy link
Owner Author

@mm2 mm2 commented on fdbfb76 Feb 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Arusekk @jonasmalacofilho @albert-astals-cid-kdab

Fine. I have created a minor revision lcms2-2.13.1 with just that fix, a version number bump in the lcms2.h header and a note in the ChangeLog. Hope that would solve the issue.
In order to minimize changes all tools, DLL and so remains with 2.13 (with no info about which revision)

Please sign in to comment.