Describe the Bug
iccBenchApply::DecodeIntent() documents itself as decoding an intent code "the way
iccApplyToLink.cpp does, so a chain given to this tool and the same chain given to that
one resolve identically". Two columns do not match, so the tool can benchmark a different
transform than the one iccApplyToLink builds — or refuse a code that tool accepts.
PR #2269 (#2268) closed the sign column and narrowed the documentation to claim only that
much. These two are the remainder.
1. The hundreds column is discarded. iccApplyToLink.cpp:1055 reads it as a luminance
request and acts on it at :1088:
nLuminance = nIntent / 100;
...
if (nLuminance) { Hint.AddHint(new CIccLuminanceMatchingHint()); }
iccBenchApply.cpp:148-149 strips the same column without reading it, and no
CIccLuminanceMatchingHint appears anywhere in the file:
nIntent = nEncoded % 1000;
nIntent = nIntent % 100; // hundreds column dropped, never assigned
2. A tens digit of 4 means different things. iccApplyToLink.cpp:1068-1070 maps it to
lookup type 0 plus a BPC hint:
case 4:
nType = 0;
Hint.AddHint(new CIccApplyBPCHint());
iccBenchApply.cpp:154 special-cases only nType == 1, so 4 reaches AddXform() as
(icXformLutType)4 — which is icXformLutBPC (IccCmm.h:136), a lookup type, not a hint.
Build Instructions
cmake -S Build/Cmake -B build -DCMAKE_BUILD_TYPE=Release
cmake --build build -j4 --target iccApplyToLink iccBenchApply
Reproduce
Measured on master e23426a0, clang-18 Release. The same code 40, one tool refuses and
the other accepts:
$ build/Tools/IccBenchApply/iccBenchApply -pixels 4096 -repeats 1 1 \
Testing/sRGB_v4_ICC_preference.icc 40
Unable to add 'Testing/sRGB_v4_ICC_preference.icc' to the chain: Invalid Look-Up Table type
$ build/Tools/IccApplyToLink/iccApplyToLink out.icc 0 2 1 t 0 1 0 0 \
Testing/sRGB_v4_ICC_preference.icc 40
LUT successfully written to 'out.icc'
For the hundreds column, iccBenchApply -perxform resolves 100 and 0 to the same
chain, as the code predicts:
code=0 -> 0 3DLut
code=100 -> 0 3DLut
Note on scope of evidence: I could not produce an observable output difference for the
hundreds column on iccApplyToLink — 100 and 0 gave a byte-identical link on
sRGB_v4_ICC_preference.icc, so CIccLuminanceMatchingHint does not bite on this profile.
The divergence for +100 is established from the code, not from a measured output delta;
a profile carrying luminance data would be needed to show it end to end. The 40 case is
measured end to end.
Expected Behavior
DecodeIntent() should read the hundreds column and map a tens digit of 4 the way
iccApplyToLink does, so the documented equivalence holds — or the two tools should stop
claiming equivalence, which is the interim position #2269 documents.
The blocker for the first option is that DecodeIntent() returns a plain
(nIntent, nType, bUseSubProfile, bUseD2BxB2DxTags) tuple and has nowhere to put a hint:
iccApplyToLink carries a CIccCreateXformHintManager through to its AddXform call,
and iccBenchApply does not build one. Adding it is a contained change to
Tools/CmdLine/IccBenchApply/, no library API change.
Practical consequence today: iccBenchApply cannot time a BPC or luminance-matched chain,
which is a gap in a tool whose purpose is measuring apply-path throughput.
I have the analysis and can open a PR for this one — it does not need a ruling the way the
sibling iccApplyToLink transform-type issue does. Say the word and I will.
Environment
- OS: Ubuntu 24.04 (WSL2), clang-18 Release, master
e23426a0
Describe the Bug
iccBenchApply::DecodeIntent()documents itself as decoding an intent code "the wayiccApplyToLink.cppdoes, so a chain given to this tool and the same chain given to thatone resolve identically". Two columns do not match, so the tool can benchmark a different
transform than the one
iccApplyToLinkbuilds — or refuse a code that tool accepts.PR #2269 (#2268) closed the sign column and narrowed the documentation to claim only that
much. These two are the remainder.
1. The hundreds column is discarded.
iccApplyToLink.cpp:1055reads it as a luminancerequest and acts on it at
:1088:iccBenchApply.cpp:148-149strips the same column without reading it, and noCIccLuminanceMatchingHintappears anywhere in the file:2. A tens digit of 4 means different things.
iccApplyToLink.cpp:1068-1070maps it tolookup type 0 plus a BPC hint:
iccBenchApply.cpp:154special-cases onlynType == 1, so4reachesAddXform()as(icXformLutType)4— which isicXformLutBPC(IccCmm.h:136), a lookup type, not a hint.Build Instructions
Reproduce
Measured on master
e23426a0, clang-18 Release. The same code40, one tool refuses andthe other accepts:
For the hundreds column,
iccBenchApply -perxformresolves100and0to the samechain, as the code predicts:
Note on scope of evidence: I could not produce an observable output difference for the
hundreds column on
iccApplyToLink—100and0gave a byte-identical link onsRGB_v4_ICC_preference.icc, soCIccLuminanceMatchingHintdoes not bite on this profile.The divergence for
+100is established from the code, not from a measured output delta;a profile carrying luminance data would be needed to show it end to end. The
40case ismeasured end to end.
Expected Behavior
DecodeIntent()should read the hundreds column and map a tens digit of 4 the wayiccApplyToLinkdoes, so the documented equivalence holds — or the two tools should stopclaiming equivalence, which is the interim position #2269 documents.
The blocker for the first option is that
DecodeIntent()returns a plain(nIntent, nType, bUseSubProfile, bUseD2BxB2DxTags)tuple and has nowhere to put a hint:iccApplyToLinkcarries aCIccCreateXformHintManagerthrough to itsAddXformcall,and
iccBenchApplydoes not build one. Adding it is a contained change toTools/CmdLine/IccBenchApply/, no library API change.Practical consequence today:
iccBenchApplycannot time a BPC or luminance-matched chain,which is a gap in a tool whose purpose is measuring apply-path throughput.
I have the analysis and can open a PR for this one — it does not need a ruling the way the
sibling
iccApplyToLinktransform-type issue does. Say the word and I will.Environment
e23426a0