This is causing some reftest failures. I've got a patch.

added a patch. Flagging vlad for review.

Comment on attachment 335954 [details] [diff] [review] patch to fix up profile handling in libpr0n Looks good, but should get joe to look at it as well.

Comment on attachment 335954 [details] [diff] [review] patch to fix up profile handling in libpr0n Looks good, but I'm wondering how it fixes anything -- it looks like you're simply removing #ifdefs? >- FloatVals->n[VX] = ToFloatDomain(In[0]); >- FloatVals->n[VY] = ToFloatDomain(In[1]); >- FloatVals->n[VZ] = ToFloatDomain(In[2]); >+ FloatVals->n[VX] = ToFloatDomain(RGB_8_TO_16(In[0])); >+ FloatVals->n[VY] = ToFloatDomain(RGB_8_TO_16(In[1])); >+ FloatVals->n[VZ] = ToFloatDomain(RGB_8_TO_16(In[2])); Unless this fixes it! What's the reason behind this?

joe - patch definitely does more than removing #ifdefs (that was just a side thing that bothered me). libpng gamma correction now also happens in the case of embedded profiles when there isn't a cHRM chunk (meaning we can't make a full ICC profile out of non-ICC chunks). The RGB_8_TO_16 was a bug in a rarely-used codepath in LCMS that came to light after I fixed the main issue. Bundling it in to avoid creating a separate bug for it.

pushed in be67522b90ad

Removing those #ifndefs means that the system libpng is required to support the various color chunks. If you try to load with a system libpng that was built without them you'll get unsatisfied references. In a stock system libpng, those PNG_NO_READ_iCCP and PNG_NO_READ_sRGB things won't be defined. I think it would be safer to leave them in.

Glenn -- I had assumed that the calls to png_get_valid(png_ptr, info_ptr, PNG_INFO_sRGB) and the like would fail if their respective #ifdefs had the feature disabled? Is this not the case? Also, if PNG_NO_READ_fOO is defined, does that mean that PNG_INFO_fOO is not defined as well (leading to compilation errors)? If so, you're right and we should file a bug for adding those #ifdefs back in.

The function won't get called but I think the loader will fuss about the unsatisfied reference anyhow.

The PNG_INFO_fOOO masks are always defined regardless of the presence of feature fOOO.

Glenn - oh right, good point. Can you open up a new new blocker for bug 455056 for this issue? I'll try to take care of it when I get back from argentina in a week.

Sure. Have a nice trip!