Closed Beep6581 closed 9 years ago
Hmm... maybe conversion should be to the working color space (working profile), not
sure if it matters though that colortemp works with with that profile or always Prophoto.
Reported by torger@ludd.ltu.se
on 2013-11-20 15:05:24
Here's a patch that removes the use of hard-coded sRGB d65 color space reference, and
instead changes to Prophoto D50
- White balance multipliers was sRGB, now Prophoto
- Thumbnails now Prophoto instead of sRGB
- Ahd/eahd demosaicers use Prophoto instead of sRGB as reference
I'm assuming we nowadays have enough bit depth in stored thumbnails to handle Prophoto.
Reported by torger@ludd.ltu.se
on 2013-11-22 11:11:41
PatchSubmitted
Reported by torger@ludd.ltu.se
on 2013-11-22 11:31:40
Same patch, with addition that working profile is ProPhoto per default.
Reported by torger@ludd.ltu.se
on 2013-11-22 12:08:24
The patch requires clearing thumbnails from cache (right-click, click from cache full,
browse to another folder, then come back).
There is still a very dramatic difference on a saturated image (or with saturation
boost) when switching working space between ProPhotoand and sRGB.
Photoshop does not exhibit the same behavior, however. If I open the same saturated
image in Photoshop and change working spaces between ProPhotoand and sRGB the image
appearance is almost constant. There is a small change in image when converting image
from PhoPhoto to sRGB in Photoshop.
It would be best for Jacques to comment on these parts of code in RT.
Reported by michaelezra000
on 2013-11-22 12:25:03
Sets prophoto as default working profile
Reported by torger@ludd.ltu.se
on 2013-11-22 12:25:49
I have not changed the behavior when changing working colorspace. A difference is expected,
as you change the range upon the tools work on.
Photoshop may have a fixed working range for their tools (eg prophoto) and thus there's
no change for them.
I don't really see the current RT behavior as a problem, as being able to select working
space is as I see it legacy stuff, and we only keep it to be compatible with old conversions
(which had sRGB working space).
From the tests I've made I see clipping when sRGB space is enabled, and no clipping
with ProPhoto. This means that if we make the tools not clip in sRGB space we break
compatibility, and if we make it clip in ProPhoto space we have gained nothing.
What I would *like* is to remove the working profile alltogether and have Prophoto
D50 as a fixed working profile, but for compatibility reasons I think it's best to
keep it for a while.
Note that we only employ the primaries part here (ie how saturated colors you can have),
the tool curves are displayed in a 2.2 gamma (ie not ProPhoto 1.8) as far as I know.
As far as I understand the only thing we see here concerning differences with the tools
is that
* sRGB clips in some tools
* ProPhoto doesn't clip [nearly as much, cameras can produce values outside but I
think it's negligible]
* As the sample range changes upon the tools operate there will be a color shift
but there's no error. You can't convert settings made in one working profile to a different.
That's sad, but I don't think it's worth the effort to fix that.
Reported by torger@ludd.ltu.se
on 2013-11-22 12:40:30
Looked around more in the code. When gammas are being used (I think it's mostly for
curves) it seems to always be an sRGB gamma, ie fixed. That's good. People would be
crazy if the curve's gamma would change.
Then there's some gamut bending going on (to fit the colors into working space), for
example in the vibrance tool.
So far the only things I see is that with a small space like sRGB you get your colors
bent, otherwise you won't. So it seems to me that with ProPhoto working profile you
can only get it to work nicer.
There may be one exception though, and I think this is where we may have a conflict
of interest -- if you have sRGB output, which is default and I think it should be,
gamut clipping will take place in the output profile step, and the algorithms used
there (rendering intent), I guess it will be LCMS doing that, may not be as good as
the gamut bending in the vibrance tools etc.
But then I think the solution would be to fix the final color space conversion instead,
ie the gamut bending should take place when converting to output profile, not before.
Reported by torger@ludd.ltu.se
on 2013-11-22 13:05:54
This is how I think the color space pipeline should work:
1. load file
2. convert to a big colorspace, which will fit the input
- default ProPhoto, that you actually can choose working profile should only be
seen as a legacy feature.
- for simplicity we can have thumbs in ProPhoto as well (with sRGB gamma)
3. apply tools within the big colorspace
4. convert based on desired rendering intent down to output color space
- output color space is often sRGB of course, should be default in profiles
- today there's no option to choose rendering intent for converting between
working space and output(?)
This is how it works today (with sRGB working profile and sRGB output)
1. load file
- white balance operates in sRGB space regardless of working profile
- thumbs operate in sRGB space regardless of input space
2. convert to sRGB but don't clip, ie we have both negative and larger than max
values
3. some tools if enabled make gamut mapping in their own ways reducing out of bounds
values to fit the sRGB working profile
- the vibrance tool gamut maps if enabled and non-zero, but not if not enabled
meaning that you can get *reduced* saturation of out-of-gamut colors by
enabling the vibrance tool
4. there is some conversion to output color space of the remaining out of gamut
colors
The current pipeline is too confusing, and it's not easy to understand when gamut bending
takes place and what rendering intent there is. It would be better to move the smart
gamut bending algorithms to the output conversion, and being able to choose rendering
intent. Or we could have it as is, but instead of checking out-of-gamut against the
working profile the tools would test against the output profile. There's just one too
much color space to care about here.
Reported by torger@ludd.ltu.se
on 2013-11-22 13:45:34
Other things to consider:
Today:
- Preview shows working space converted to monitor space (output space not involved)
- Clipping indicators output space
- Histograms show working space
I think preview and clipping is as it should be (I'd like to add a "soft proof" feature
some time in the preview so you can filter through output profile too, but I've noted
that in a separate enhancement issue).
However that the histogram shows the working space rather than the output space may
be a bit confusing, as if you have a large working space and a small output space (typical
with the model I suggest, unlike both sRGB) the histogram may not show clipping while
clipping indicators do. I think the histogram should show the output space.
Reported by torger@ludd.ltu.se
on 2013-11-22 17:26:46
Added to the patch so the histogram shows the output color space.
It would have been a one-liner, but had to make a function that converts an ICC profile
to our standard gamma 569/256=2.22 so histogram won't shift if you choose an output
color space with a different gamma.
Reported by torger@ludd.ltu.se
on 2013-11-22 18:33:53
I think it would be useful to leave these three items as-is by default, but enhance
all of them with inclusion of the output profile when "soft proof" check-box is checked.
Reported by michaelezra000
on 2013-11-22 18:33:58
I still think that histogram = output profile should be default. I'd like it to be that
99% of the users don't need to care about working profile, ever.
If the histogram shows working space and default is Prophoto work and sRGB output (which
I think it should be) it will be very noticable that you clip output without clipping
histogram. It will also be noticable that clipping indicators and histogram don't match.
This has not been a problem when default is work=output=sRGB, but I which often work
with the ProPhoto/sRGB combination has noted this problem.
With the colorspace strategy I'd like to introduce with this patch there's no purpose
of seeing the working space histogram either. All you need to know about the working
space is that it's large enough to fit your input and output spaces.
Reported by torger@ludd.ltu.se
on 2013-11-22 18:49:57
About the "Avoid color shift" function of Lab and Vibrance: if I understand it correctly
the purpose of this function is to desaturate out of gamut colors to avoid a single
channel clip. The purpose is similar to "rendering intent" of icc standard, ie a method
to reduce the gamut.
When using Prophoto as work space this setting makes no difference in practice since
no colors are pushed out of gamut. I think they should relate to the *output space*
instead of working space, as the purpose of them would be to fit the colors into the
space you are making output for.
(By some reason I don't yet understand the rendering intent setting makes no difference,
it could be that out of gamut colors have already been clipped when LCMS is called.)
Reported by torger@ludd.ltu.se
on 2013-11-22 19:22:06
The working space is very close to not matter at all, only a few tools care about the
gamut limits. So an alternative approach could be to strive for making rendering the
same regardless of working space, and when it is we just remove the setting, and use
something fixed internally.
(I noted that LCMS conversion to output space in the end is 16 to 16 bit, with a huge
color space there can be precision loss, so it should probably be change to a float)
Reported by torger@ludd.ltu.se
on 2013-11-22 22:04:24
Added issue 2065 fix, LCMS output color space conversion now floating point (and a little
faster).
Reported by torger@ludd.ltu.se
on 2013-11-23 10:07:34
Something is messed up in my last patch, got bad render when opening a floating point
tiff. Anyway, probably best to split the patch to target specific issues, just wanted
to make a "what if" test for prophoto.
Reported by torger@ludd.ltu.se
on 2013-11-24 13:56:27
A little bit cleaned up patch, removed some LCMS stuff not needed. The messed up thing
seem to be gone too.
Looked into if I could have thumbs still in sRGB, but it will complicate the patch.
Since this makes whitebalance ProPhoto-related and thumbs use whitebalance too it's
easier to have them in Prophoto.
Reported by torger@ludd.ltu.se
on 2013-11-25 09:18:20
I do not think it's a good idea to replace "sRGB" with "Prophoto" for white balance.
:)
I tried this change with Dcraw there 5 years, and the result was not good. At the time
I found documentation (I can not find it - Adobe ??) that said that the process for
calculating multipliers channels should be based on "srgbD65".
I just tried the patch and for me, this is not good.
While the image is identical for "Camera" with and without patch, but for example a
camera setting "sunny" 3900K is obtained with the patch, whereas without the patch
it is 4950K (4500 < sunny <5500), we has the same distance for a shade image (5000K
instead of 6000K)
By cons, for me it is desirable to replace "working profile" : default sRGB by default
Prophoto.
:)
Reported by jdesmis
on 2013-11-26 12:39:37
Jacques, wellcome back. Wishes for your son also :)
"I tried this change with Dcraw there 5 years, and the result was not good. At the
time I found documentation (I can not find it - Adobe ??) that said that the process
for calculating multipliers channels should be based on "srgbD65"."
So we should revert any changes in dcraw.cc matrices which are probaly at a different
illuminant than D65. I mean those by Huelight ..
Remember the Panasonic underwater ..
Edit. I just discovered that someone (torger ?) already reverted all matrices to dcraw's
(Adobe's) default D65 ..
On the other side if we adapt (Bradford) those D65 matrices to D50, for use with Prophoto
what will happen ?. Or use a wider than sRGB colorspace (D65 again) ... like Prophoto
D65 (Dcraw saves tiffs at this) ?.
Reported by iliasgiarimis
on 2013-11-26 13:19:40
(I have not reverted the custom matrices, they are in rawimage.cc or camconst.json,
just reduced the number of changes we do to dcraw to reduce maintenance work.)
May be some bug left in the white balance stuff, so the temperature shift could be
a bug rather than just a colorspace issue, not sure.
I'm okay with sRGB white balance reference if we can handle white balance range of
cameras, which seems to be a problem now. I can't use the auto white balance engine
for the pre-demosaic reference wb as it can't handle the full range of the camera.
Maybe it could be possible to use camera primaries for white balance model?
Adobe don't do sRGB these days, they do Prophoto D50 throughout, it's their reference
space. Some white balance calculations is quite well specified in the DNG specification
(sort of) and sample code how they work is in the DNG SDK.
Having *working space* in sRGB is the largest limitation today though I think, as tone
curves hard-clip and some tools care about out of gamut colors and others don't, so
it gets very confusing. I always switch to Prophoto as working space before I start
work with an image to avoid clip away colors from my camera.
Reported by torger@ludd.ltu.se
on 2013-11-26 13:55:55
Hello Ilias
No the change only affects rmul, gmul, bmul in colortemp.cc and not the others...
rmul = sRGBd65_xyz[0][0]*Xwb*adj + sRGBd65_xyz[0][1]*Ywb + sRGBd65_xyz[0][2]*Zwb/adj;
// Jacques' empirical modification 5/2013
gmul = sRGBd65_xyz[1][0]*Xwb + sRGBd65_xyz[1][1]*Ywb + sRGBd65_xyz[1][2]*Zwb;
bmul = sRGBd65_xyz[2][0]*Xwb*adj + sRGBd65_xyz[2][1]*Ywb + sRGBd65_xyz[2][2]*Zwb/adj;
:)
Reported by jdesmis
on 2013-11-26 14:01:40
Issue 2053 has been merged into this issue.
Reported by torger@ludd.ltu.se
on 2013-11-26 14:05:21
A minor change for 4.1 might be to set default working space in profiles to ProPhoto
but make no other change.
That will avoid premature clipping of camera colors. It's primarily that thing I find
to be a bit ugly.
Reported by torger@ludd.ltu.se
on 2013-11-26 14:09:00
Hello Torger
I think it should not be confused, work prophoto (working profile) and calculate multipliers
channels (white balance)
I totally agree to use (like Lightroom) Prophoto as "working profiles", but for white
balance it is imperative if we used prophoto instead of having sRGB that values of
temperatures, with substantially the same as "sRGB" and "prophoto" (sunny #5000K, cloudy
#6000, shade #8000, etc.)
:)
Reported by jdesmis
on 2013-11-26 14:10:21
Issue 2053 has been merged into this issue.
Reported by jdesmis
on 2013-11-26 14:15:48
Torger
To whom you speak ? (me ?), my English is very bad ...
But in summary, I agree entirely as prophoto for "working profile"
:)
Reported by jdesmis
on 2013-11-26 14:18:22
In this patch I've removed all changes to the code, and only change the profiles working
space.
Reported by torger@ludd.ltu.se
on 2013-11-26 15:31:20
Brave endeavor. This part in RT is most certainly the hardest part to get right and
by far the most important part to get right. Emil has done a lot of work and research
on this in the past. He might be able to give more info on choices made in the code
about gamut and color temperature (D50/D65).
Reported by janrinze
on 2013-11-27 23:43:56
Brave endeavor. This part in RT is most certainly the hardest part to get right and
by far the most important part to get right. Emil has done a lot of work and research
on this in the past. He might be able to give more info on choices made in the code
about gamut and color temperature (D50/D65).
Reported by janrinze
on 2013-11-27 23:43:56
I think we can start with change default working profile, it seems to be a safe change
and also takes away all major drawbacks with current small space reference.
Another change that I think could be nice to have now (which I've already coded in
one of the patches) is to change the histogram so it relates to output space rather
than working space. If we change to prophoto default working with sRGB output it will
become evident that there is a mismatch. Clipping indicators already seems to be related
to output space and since you generally work to fit your input gamut to your output
space it seems logical to me to show output space in the histogram.
Another alternative is to combine it with the proofing feature, ie when you enable
a proofing checkbox the preview image filters via the output profile and histogram
changes from showing working space to output space. Today the preview image shows "as
much as possible" ie the working space transformed to display space (more than sRGB
if you have a wide gamut screen), which also is what other converters do so I think
it's the right default. I think I prefer to always have histogram at output space,
but I wont fight for it if others want otherwise.
Reported by torger@ludd.ltu.se
on 2013-11-28 12:24:47
I'd also prefer to always have histogram at output space, I think it will be the best
(and least confusing) choice for most people and most situations.
Reported by stefan.ittner
on 2013-12-01 19:49:45
If we are to act, we must act now. If WP gets changes, let's do it today so we have
more time to have it tested in the wild, and to revert to sRGB if it causes issues
which can't be resolved soon.
Torger: I would also like the main histogram to be representative of the end result.
Reported by entertheyoni
on 2013-12-04 23:49:01
Committed to get more testing
"issue 2068, WorkingProfile changed to ProPhoto to prevent color gamut clipping at
the beginning"
Reported by entertheyoni
on 2013-12-05 02:09:45
I'm a bit swamped with work right now, I'll see what I can do.
My idea for action to 4.1 would be to change profiles to prophoto working and make
histogram represent output space. It's a small patch which is complete in but in parts
so I need to assemble it.
Reported by torger@ludd.ltu.se
on 2013-12-06 06:37:09
Quick assembly in the morning; here's a patch for making output space the histogram
space. Also changed default working space to prophoto in procparams.
Reported by torger@ludd.ltu.se
on 2013-12-06 07:05:53
Works great! Thanks, Anders:)
Reported by michaelezra000
on 2013-12-06 22:32:43
Hi Andres, I can confirm that the patch compiles and changes the histogram when changing
output profile, I don't know how to confirm whether what it shows is actually correct.
http://i.imgur.com/9lijGIE.png
http://i.imgur.com/zcPPyBR.png
http://i.imgur.com/QABQMzM.png
Reported by entertheyoni
on 2013-12-06 23:50:32
If the histogram takes the output profile into account, shouldn't the preview too?
Reported by entertheyoni
on 2013-12-06 23:52:09
#38 seems correct. Easier to verify if you expose your saturated images brighter so
it definitely clips on the right side.
#39: that's soft-proofing. I do think it's logical to do as you say, but "every" color
managed software I've tested puts the full color gamut to the screen (ie the screen
limits it) unless you enable soft-proofing.
The thing with soft-proofing however is that it's often combined with paper simulation
and stuff which makes the visible appearence quite different. Only applying the output
profile will make only a small difference, not visible for most cases and images (you
need sRGB output space, wide gamut monitor and a saturated image to see a difference),
so one could argue that it's most logical and won't do any harm to always filter through
the output space before sending to the monitor.
Anyway, I think we can stay low on using output space in the preview for now, and come
back to it later, and then think about if it should always be enabled or only if soft-proofing.
Reported by torger@ludd.ltu.se
on 2013-12-07 16:43:18
Can we commit the patch(es)?
In head, applying Neutral to a photo sets WP=sRGB and OS=NoICM. Does the patch fix
that too?
Reported by entertheyoni
on 2013-12-09 15:21:52
Yes this patch fixes this too.
Committed it to default.
Looking through this issue if it needs splitting (will then add separate issues), renamed
this one to only represent the committed patches.
Reported by torger@ludd.ltu.se
on 2013-12-09 17:31:19
FixedPendingConfirmation
Can we set this to fixed? I'm satisfied for 4.1. After release we can think about adding
soft-proofing etc.
To make it clear, the patch is supposed to have changed the following:
- histogram always show output space, which also should be matching the preview
clipping indicators
- bundled profiles have Prophoto as default working space
Reported by torger@ludd.ltu.se
on 2013-12-18 06:59:11
Fine for me. Has the manual been updated to reflect the changes?
Reported by entertheyoni
on 2013-12-18 22:15:39
Yes did that yesterday in the color management section. And as the previous default
working space was same as default output space (sRGB) the many mentions of the histogram
related clipping to clipping in output space already so that needs no change.
Reported by torger@ludd.ltu.se
on 2013-12-19 08:39:18
Fixed
Originally reported on Google Code with ID 2068
Reported by
torger@ludd.ltu.se
on 2013-11-20 14:59:50