Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.81k stars 318 forks source link

Rendering intent, monitor profile & soft-proofing #2744

Closed Beep6581 closed 8 years ago

Beep6581 commented 9 years ago

Originally reported on Google Code with ID 2763

Rendering intent depends on the particular image and need. If you need accurate color
reproduction, or if you work on a dull image, you need to use the "relative colorimetric"
rendering intent to preserve color accuracy. If you work on a photo of a deep red rose,
you want "perceptual" to not clip the reds. Therefore this option belongs in each PP3
and in the tool panel, not in Preferences.

Reported by entertheyoni on 2015-05-03 15:51:32

Beep6581 commented 9 years ago
I agree. However, I wonder if rendering intent actually does anything in RT? I tested
with a CC24 shot and saved 4 pictures with all the different rendering intents.

Processing settings:
Neutral, WB on second grey patch, +50 Exp saturation +50 LAB saturation (to properly
blow the colors :))

I changed the rendering intent option and re-started RT between each processing. I
don't see any difference at all between the result. Shouldn't this be easily visible
for these extreme settings with all the blown colors?

Files are here: http://filebin.net/qlfbiyd37w

Branch: default
Version: 4.2.162
Changeset: 4ae33876d7aa
Compiler: gcc 4.9.2
Processor: generic x86
System: Windows
Bit depth: 64 bits
Gtkmm: V2.22.0
Build type: Release
Build flags: -m64 -mwin32 -mthreads -msse -msse2 -mtune=generic -fopenmp -Werror=unknown-pragmas
-mwindows -DNDEBUG -O3 -Wno-aggressive-loop-optimizations
Link flags: -mwin32 -mthreads -static-libgcc  -mtune=generic -mwindows -s -O3
OpenMP support: ON
MMAP support: ON

Reported by stefan.ittner on 2015-05-03 18:27:12

Beep6581 commented 9 years ago
I too have noticed rendering intent not having any effect on saved pictures.

RT 4.2.1 for Windows Vista/7/8 64-bit

Reported by trr@thomasrutter.com on 2015-06-03 03:09:35

Beep6581 commented 9 years ago

Rendering intent has been removed, closing.

cipriancraciun commented 9 years ago

Wasn't the "rendering intent" supposed to change how the image is displayed, based on the current selected monitor profile?

Thus indeed on some photos one would like to have color accuracy, and on some others he prefers to have finer shades, therefore moving it in the processing profile might be OK.

However removing it completely is a bad idea, for the simple reason that now, and I'm assuming that the relativ colorimetric intent is used by default with all, on my poor Lenovo laptop LCD it makes almost impossible to process any photos...

Please add this option back somewhere...

cipriancraciun commented 9 years ago

The newer variant that I tested which doesn't permit anymore to choose the rendering intent, is 4.2.366, and the older one that I previously used and which allows this choice is 4.2.274.

As expected in 4.2.274, by using the rendering intent of "perceptual" I can do my normal processing and the monitor displays colors as best it can.

However under 4.2.336, all the colors that are out of the gamut of my monitor, are crushed to either black or some murky shade of color, which as said makes using Rawtherapee impossible.

As said earlier please revert this commit as it seriously breaks Rawtherapee.

Hombre57 commented 9 years ago

I second @cipriancraciun comment. Instead of removing it, we should have moved it in the ICM panel as suggested in original comment and make it work for booth preview image and output image. Rendering intent is a "must have" for this kind of software.

Beep6581 commented 9 years ago

@cipriancraciun please upload a sample raw file + monitor ICC + PP3 http://filebin.net/

cipriancraciun commented 9 years ago

Before you get to the uploaded files I want to give my input on how this rendering intent feature might be best suited for some workflows.


Due to the small gamut of my laptop monitor, when I use it I must be able to use "perceptual" rendering intent (eventhough the colors will off) for all my images, else most of the saturated colors will be squashed into a single one. However when using an external monitor I can use "relative colorimetric" especially if the monitor has a wide gamut.

Therefore although it is useful to have the rendering intent per image processing profile, there should be a way to temporarily override this globally for all images, but without touching the image processing profile.

The use case stems from the problem described above, as for example, say I usually work with the external monitor, and most of the images are processed with "relative colorimetric" intent; however when on the road if I open the catalogue or try minor enhancements I'll get the awful color squashing, and the only way to fix that is to change all the images to now use "perceptual"; then only to have to revert all this when I return to my monitor.

As such I think the best approach for such a feature is either:

(A) leave it as it was, as a global option in the color management tab, with the default of "perceptual";

(B) allow the user to choose per image one of the rendering intents, perhaps with the default also of "perceptual"; but also have the global option in the color management tab, with a choice of "as configured per image" and one of the four rendering intents, which when chosen will override (but not change) the choice per image;


I have uploaded the requested files at the following link:

http://data.volution.ro/ciprian/665371e6adeefdbb704aea9ed29f10e7/02

The issue that I was describing I assume should be visible if you load my profile as the monitor profile, and if you use "relative colorimetric", which in my monitor's case will crush many saturated colors into a few colors. (It is highly likely that all the colors will seem wrong due to the mismatch between my profile and your monitor, but you should easily see the loss of contrast and the color squashing.)

Moreover please note that the image above uses a camera profile (also uploaded), which if not used the image will be underexposed and the colors will be under-saturated. Thus either use that DCP profile, or raise the exposure and saturation.

Hombre57 commented 9 years ago

We have 2 targets regarding color management : 1- the output profile with its own color space and rendering intent (that why we need a "Rendering intent" in the ICM panel), 2- the monitor display (that what was used the "Rendering intent" from Preferences).

Having both is having soft-proofing, i.e. you either convert the image to the monitor target, or you convert it to the output file color space then convert it a second time to the monitor display color space.

Having none of them is a no go for most photographers, having both "would be nice". If we don't want to offer softproofing, we should at least add a "Rendering intent" button in the ICM panel, and use it for the output file, but we won't be able to preview that (as we can't preview the output colorspace neither for now).

Beep6581 commented 9 years ago

The choice of rendering intent for the conversion of working->output belongs in the PP3 as the way you use the tools will heavily depend on that. The intent used for conversion to the monitor space belongs outside of the PP3, but in the Editor tab where it's easily accessible. I would also like the the monitor profile chooser to be moved into the Editor tab where it's actually usable for those who have two or more monitors, so I don't have to surf through Preferences every time I move the RT window from one monitor to the other.

Hombre57 commented 9 years ago

I used to think that each monitor as a text identifier (and certainlly number as well). If we can read the monitor's name (and I think it's possible https://developer.gnome.org/gtkmm/unstable/classGdk_1_1Display.html#a81036bff74e1bb09cdd230743d433ec7 ), then we should be able to catch events saying that the window is displayed on a different display, and automatically select an associated profile. Association should still be done in Preference, in this case.

I don't know how to deal with configuration using 2 identical monitor though.

cipriancraciun commented 8 years ago

Any updates on this one? (The newly released 4.2.447 still seems to not have the option to choose the rendering intent in the Color Management preferences.)

Beep6581 commented 8 years ago

I'm working on it when I have time. It will take time.

cipriancraciun commented 8 years ago

I'm sorry if this sounded like "pushing". (I know this is an open source project, on which development is done in spare time and for no payment, and I'm grateful. Moreover RawTherapee is one of the few RAW processing software that works on Linux, and it is great one even.)

I asked because I thought that perhaps a first step in solving this issue would be to simply revert the commit that removed the option in the first place. Then as time permits the other options would be introduced.

Could you perhaps point me to the commit in question so I can "un-patch" the current branch and see if that would work?

Thanks.

cipriancraciun commented 8 years ago

After a very rough look through the RawTherapee source code (and the large patch that removed the rendering intent choice from the UI), I think the following simple patch might solve the issue for those that want the perceptual rendering intent for the image display (and are able to re-build RawTherapee from source).

I have visually compared the displayed image (in RawTherapee "develop" view) for both this patched version and the latest one that worked correctly (i.e. 4.2.274) and I don't see any difference, thus I assume my patch worked correctly. (I have not processed any image yet, but I doubt it will have any impact on the final rendered JPEG.)

Thanks for this nice software, hopefully soon we'll have the rendering intent choice back in the UI.

(The patch is based on the latest master branch. I just looked in the code for the usage of the cmsCreateTransform function, and where I saw that a monitor profile was used, currently only in improcfun.cc, and then I replaced the rendering intent with a newly introduced constant rt_MONITOR_RENDERING_INTENT, which I defined in improcfun.h as being INTENT_PERCEPTUAL. I would suggest the developers that handle the color management code to introduce such "alias" constants so that one can easily track what is the intended usage, like for example rt_SOFT_PROF_RENDERING_INTENT or rt_SAVE_RENDERING_INTENT, etc. as there are quite a few places in the code where the said function is used, and at a cursory look it is hard to identify the purpose of the color space transformation.)

diff --git a/rtengine/improcfun.cc b/rtengine/improcfun.cc
index 88ed3f7..af3d912 100644
--- a/rtengine/improcfun.cc
+++ b/rtengine/improcfun.cc
@@ -223,7 +223,7 @@ void ImProcFunctions::firstAnalysis (Imagefloat* original, const ProcParams* par
     if (monitor) {
         lcmsMutex->lock ();
         cmsHPROFILE iprof  = cmsCreateLab4Profile(NULL);
-        monitorTransform = cmsCreateTransform (iprof, TYPE_Lab_FLT, monitor, TYPE_RGB_8, INTENT_RELATIVE_COLORIMETRIC,
+        monitorTransform = cmsCreateTransform (iprof, TYPE_Lab_FLT, monitor, TYPE_RGB_8, rt_MONITOR_RENDERING_INTENT,
                                                cmsFLAGS_NOOPTIMIZE | cmsFLAGS_NOCACHE );  // NOCACHE is for thread safety, NOOPTIMIZE for precision

         Glib::ustring outputProfile;
@@ -236,7 +236,7 @@ void ImProcFunctions::firstAnalysis (Imagefloat* original, const ProcParams* par
                 lab2outputTransform = cmsCreateTransform (iprof, TYPE_Lab_FLT, jprof, TYPE_RGB_FLT, INTENT_RELATIVE_COLORIMETRIC, cmsFLAGS_NOOPTIMIZE | cmsFLAGS_NOCACHE );

                 if (monitor) {
-                    output2monitorTransform = cmsCreateTransform (jprof, TYPE_RGB_FLT, monitor, TYPE_RGB_8, INTENT_RELATIVE_COLORIMETRIC, cmsFLAGS_NOOPTIMIZE | cmsFLAGS_NOCACHE );
+                    output2monitorTransform = cmsCreateTransform (jprof, TYPE_RGB_FLT, monitor, TYPE_RGB_8, rt_MONITOR_RENDERING_INTENT, cmsFLAGS_NOOPTIMIZE | cmsFLAGS_NOCACHE );
                 }
             }
         }
diff --git a/rtengine/improcfun.h b/rtengine/improcfun.h
index cfabbba..24f292b 100644
--- a/rtengine/improcfun.h
+++ b/rtengine/improcfun.h
@@ -48,6 +48,8 @@ PIX_SORT(pp[4],pp[7]); PIX_SORT(pp[4],pp[2]); PIX_SORT(pp[6],pp[4]); \
 PIX_SORT(pp[4],pp[2]); median=pp[4];} //pp4 = median

+#define rt_MONITOR_RENDERING_INTENT INTENT_PERCEPTUAL
+

 namespace rtengine
 {

P.S.: For some reason (on latest ArchLinux) I had to append an add_definitions(-std=c++11) to CMakeList.txt, else the build would fail with a strange GCC (5.2.0) compiler error saying that if I wanted to use "extended" features I hand to target this C++ language standard.

adamreichold commented 8 years ago

It certainly won't close this bug, but maybe the branch could be a stopgap measure for those users that this to work with their various monitors but cannot build from source. It is basically just @cipriancraciun's change with the option added back into the preferences dialog but below the monitor profile to indicate that it is only relevant there.

@cipriancraciun I think the compiler flag is necessary since libsigc++ 2.0 or later mandate C++11 support and RT currently does not pick up the mandated compiler flags from this libraries pkg-config module.

adamreichold commented 8 years ago

As discussed on IRC, I tried to inch closer to a more useful solution by making the monitor profile and intent quickly selectable using combo boxes in the editor panel as suggested. The branch is now in a state where I would be glad about some advice.

Besides smaller issues with missing translations and GUI formatting, my main problem is whether using the ProcParams to temporarily and optionally override the global settings is alright or whether the monitor profile should actually become a persistent parameter.

I also opted to use the existing ICC directory for the available monitor profiles instead of adding an additional setting which might be considered more useful and it seems a bit inconsistent that the default monitor profile can be chosen using a full path but its temporary overrides come from the default ICC directory. Also I am not sure whether adding the ability to select an arbitrary file from the editor panel is really useful in addition to selecting one from the default directory.

Personally, I would argue that the monitor profile should be coming from the same directory as all other ICC profiles. I would also like to enable recursive scanning of the ICC directory again, but with depth limited to two or three levels to avoid the problems of (Google code) issue #1730 and present the user with a hierarchy of selections in the profile combo boxes instead of a flat list.

And of course, the rendering intent used for the output profile is still not implemented yet...

Beep6581 commented 8 years ago

! :) Your fast progress is great to see!

The preview rendering intent should not be stored in the PP3. The working->output space rendering intent, if we have one, should be stored in the PP3. As such, we don't need functionality to override these from Preferences, and I think that would not be intuitive even. Preferences would just serve to point RT to the right folder which contains the monitor ICCs.

One thing I wanted to have, as this has caused massive confusion in various places I looked online, is a function to check which rendering intents are supported by the destination color profile (or is it both color profiles in question? I think it's just the target one, and in this case target means monitor). I don't think there is such a function in RT currently. We only need to provide two rendering intent choices: relative colorimetric and perceptual. The other two are irrelevant to photography. As far as I know, LUT profiles can support both rendering intents of interest to us, but matrix profiles support only relative colorimetric. It would be very good if the unsupported intent was grayed-out (done while loading the target ICC), otherwise I expect this would cause confusion and frequent invalid bug reports. If RawTherapee did have this supported rendering intent-checking function, it would be a big plus as I'm unaware of any other program having this functionality, including Photoshop (though I'm unfamiliar with the newest PS).

michaelezra commented 8 years ago

My 2c on placement of combo for monitor profile:

Beep6581 commented 8 years ago

I don't mind if RT automatically selects the monitor ICC which corresponds to the "loaded" ICC profile of the screen it's on (typically done via the _ICC_PROFILE X11 atom), but I need to be able to override it with a single click (mouse scroll-wheel over combobox). There are legitimate reasons for having several profiles for a single monitor, all of them correct, all of them designed for a different purpose. There are also situations where a user may have a monitor profile loaded system-wide but want to use a different one in RT, and change them on-the-fly.

michaelezra commented 8 years ago

I am puzzled why... monitor profile is setting for the monitor, thus is system setting, not RT.

cipriancraciun commented 8 years ago

The very last comment by @michaelezra confused me a bit, and prompted me to once more raise my initial concern that made me comment on this issue in the first place, namely:

Regardless on how this issue is solved, please make sure that the user has a way to (easily) choose both the monitor profile and its rendering intent.

I would prefer to have it (like it was) in the global RawTherapee preferences menu, and its choice not to reflect into the PP3 (which might get shared between multiple machines with different displays). It might be lovely to additionally have the option, while editing the image, to change the monitor profile and intent, right below the color management tab (with the defaults taken from the global options). It would be even better if additionally RawTherapee would try to detect it from the display properties, but let me override it. I would even live with having to export strange environment variables to make it happen. As long as I have a way to do so.

Just making sure the option will be there somewhere. :)

Beep6581 commented 8 years ago

Michael, Ciprian: a monitor profile needs to work in tandem with calibration curves which are loaded in the video card's gamma table. These curves can and often are but don't have to be stored in the profile itself. These are applied to everything on-screen. Now separate from them you have the actual monitor color profile. It can be applied to a single window or a single drawable surface, it does not have to be applied (or even cannot be applied) to the whole monitor. Desktop environments often let you specify a monitor profile and then programs which know how to use it look in one of the standard locations to find it. In X11, the standard location is the _ICC_PROFILE atom. In Windows I suppose its some registry key. For convenience, I have nothing against RT automatically checking these locations to preselect in the combobox the currently-set system-wide profile, if any, but I require a way to easily override this, and that is by having a combobox accessible under the preview. The chosen monitor profile and its rendering intent should not be stored in the PP3, is it not an image processing setting.

michaelezra commented 8 years ago

For what purpose would one select a monitor profile in RT that is different from the system settings? Why overlay multiple profiles for the same device at the same time? I don't object using monitor icc combo in editor (if there is space for it), but I would only use it when working in multiple displays and if RT cannot auto-detect system settings. Any competent user who would think about a monitor profile, would certainly have it setup in the system settings. Of course it would be smoothest user experience if RT just picked it up. This is the way Adobe products were recently changed. I should to say it was confusing at first, as monitor profile combo used to be available and then it was removed entirely, as software is intelligent now to make it work just right.

Beep6581 commented 8 years ago

Random example: System-wide (again, there is no such thing as "system-wide". By it I mean "in any program which is designed to detect my selected system monitor profile"), I may want to use a profile designed to use my monitor's native white point and richest blacks along with perceptual rendering intent with linear calibration curves, while in RT for some photo I may want to use a profile with D65 and black point compensation and using relative colorimetric rendering intent with calibration curves applied. A user shouldn't need to scramble through system settings and restart RT to do this.

"Any competent user who would think about a monitor profile, would certainly have it setup in the system settings" I have three issues with that. First, I would rephrase that as "typical user". Competent users may be more demanding. Second, "it" is not necessarily correct, as there may be more than one valid monitor profile for a single monitor, each designed for a specific purpose, as I prophylactically wrote before. Third, there may not be a system settings.

http://dispcalgui.hoech.net/ http://www.argyllcms.com/doc/Scenarios.html#PM1 http://ninedegreesbelow.com/photography/monitor-profile-calibrate-confuse.html

cipriancraciun commented 8 years ago

I am well aware that for a fully color managed solution the usage of a display's profile is not enough, and it has to be used in conjunction with properly calibrated video card and display.

Moreover I understand that for "normal" users, the desktop environment already properly configures the profile and RawTherapee is able to correctly detect and use it. (Although I am quite certain that 99% of the "normal" users don't even have a calibrated and profiled display. Thus I would say that the "normal" user doesn't even need color management enabled.)

Unfortunately some don't have "normal" setups, thus I presumed that even environment variables would suffice to let those users (that really need or want) to override RawTherapee's profile and rendering intent. (Moreover, if I'm the only user of such a feature, we can just close this issue and not waste any more time that can be spent on other features, as I can easily patch RawTherapee myself and use a custom build from now on.)

As for an example where RawTherapee's profile doesn't match the system it is running on: think about using RawTherapee via VNC or remote X server, where the remote display is already calibrated and one just needs to instruct RawTherapee to use that display's profile. (I sometimes use such a setup with a different laptop that has a better monitor than my own. Basically I use the second laptop display and keyboard, but my own laptop processor and file-system.)

However even if you remove (from the UI) the option to override the monitor profile, removing the rendering intent would be an extremely bad idea, because most laptops have such a bad display (mine included) that the relative colorimetric rendering intent (the current default) makes RawTherapee unusable.

Hombre57 commented 8 years ago

@cipriancraciun

Don't worry, we all want to have this Monitor profile + Rendering Intent. The question is about how far to make things automagical, but as of now, we can just keep what was already there in RT, i.e. a button to detect the system's default monitor profile, and being able to override it. A better GUI implementation would be to add an "system defined" entry in the combo box for systems that support it, and make the value of this combobox persistent through options as Adam already did.

Then the rendering intent combobox could be replaced by our custom PopUpButton widget (unless you can find one in Gtk that works the same). In this case, we have to create 4 icons (one per rendering intent) that will be displayed in the button.

@adamreichold

You switch the rendering intent from a combobox to a checkbox, but I think that it's confusing, at least without tooltip. I'd rather use a PupUpButton instead, and put the 2 (if not 4) different intent.

I'm also investigating why I don't see any difference here when changing the intent.

Beep6581 commented 8 years ago

@Hombre57 you don't see differences probably because your monitor profile is a matrix one and/or does not have gamut mappings for the perceptual intent.

I can make the icons if need be. Where is this custom PopUpButton widget used and why not use a combobox?

Hombre57 commented 8 years ago

PopUpToggleButton is used for selecting curve's type. PopUpButton should be the same, but without the toggling function. It's still unused for now. It think there's a bug somewhere in this class because RT crash. That's why my patch still use MyComboBox widgets.

For the icons, it's too late, I've done them :) but you can do better one if ever.

Hombre57 commented 8 years ago

The Monitor profile and soft-proofing branch has been updated. It should be complete. We could still enhance it by using lcms' soft-proofing profile/feature, but someone else will have to do it.

As you can see, the Rendering intent is a graphical button for the monitor profile and a combobox for the Output rendering intent. I can do a graphical button too, but i guess that it have to stay sensitive even when using the manual output gamma. Should I add it below the Output profile combobox or on its right ?

And last but not least, would it be interesting to support lcms' own "black preservation" rendering intent ?

adamreichold commented 8 years ago

@Hombre57 Could you open a new pull request for add-monitor-profile-and-softproofing so that we can test and discuss the code? (Also I think the last two commits would benefit from being squashed but this is something to discuss in the PR.)

Hombre57 commented 8 years ago

PR #3029 created.

What do you mean by "squashed" ?

adamreichold commented 8 years ago

The changes from both commits being merged into a single commit (since the second is only clean up for the first), which is done using git rebase -i.

Hombre57 commented 8 years ago

When you rebase like that, what am I supposed to do to include the rebasing ? You already did that in your branch and had to delete the whole local branch and import again, otherwise there was conflicts when just pulling.

adamreichold commented 8 years ago

Yes, this prevents pulling but just deleting the local branch and checking out the remote branch again is fine since the branches basically share only their name. Another possibility is to do git fetch and then git reset --hard origin/branch-that-was-rebased after checking out the local branch. (In this regard, rebasing does it make it harder to just update local copies of these branches, but IMHO it is essential in providing a clean history which others can use to read up on the actual changes in the future, even after long code reviews and several changes to the same place in the code.)

Beep6581 commented 8 years ago

I will discuss usability here, while leaving the PR for you to discuss code.

Some requests:

  1. Could you make the new intent button react to shift+mouse-scrollwheel as the combobox does?
  2. I'm a bit confused about previewing the output profile. At some time in the recent past, @atorger made is to that the choice of output profile was instantly visible in the preview. For some weeks now I have been unable to make that work - when switching from RT_sRGB to RT_Medium_gsRGB there is no difference in the preview, while the difference should be very clear on a photo with strong colors. The histogram changes, the preview does not.
  3. The Soft-proof button does nothing, how do I use it?
  4. The Soft-proof button's icon is nice, but the Intent button's icons can be improved, specifically the Relative Colorimetric icon seems wrong. I'll send suggestions soon.
Hombre57 commented 8 years ago
  1. I'll try
  2. The output color profile and intent is now shown with soft-proofing on only. So is it working in this condition ? If this was the default behavior before, we could make the soft-proof state persistent in options.rtSettings, so the user would only have to activate it once ? The value would be the one of the last click on any of this button
  3. well... it does nothing. Maybe I should rework the PopUpButton to popup the menu when clicking the button. To save some space, I was about to use a PopUpToggleButton, so that when toggled on, it would enable soft-proofing, but it's not very intuitive.
  4. Yes, I find it nice too, but i little bit too bright. Could you increase it's transparency while you're at it ? For the rendering intent, I tried to understand and show in the icon how the values would be modified. Not sure that I correctly understood all subtleties between the options. I'm waiting on your versions.

Btw, is there an interest in offer the lcms custom intents as well (with black preservation) ?

Beep6581 commented 8 years ago

About rendering intent choice: what I wrote about only "perceptual" and "relative colorimetric" being of interest to photographers is true, but I since learned that those gamut mappings are only nominal - the actual mapping could be anything. This allows one to abuse be creative with the ICC file and store any mapping of interest under any of those names. As such, I must reverse my position and ask that RT supports not only relative (the default) and perceptual but also saturation.

This is from Florian Hoch, author of dispcalGUI:

LUT-type profiles can contain one or three tables. If only one table is present (table 0), this is the (relative) 'colorimetric' table. If there are three tables, table 0 is the 'perceptual' table, table 1 is the (relative) 'colorimetric' table, and table 2 is the 'saturation' table. There may be just one or two actually unique tables in the profile, and the other(s) just a pointer to the unique one(s).

Absolute colorimetric is derived from the relative colorimetric data by applying a chromatic adaptation transform (or, rather, undoing it): All color data in a profile is stored adapted to CIE D50 as per the ICC spec.

It is generally advisable that the colorimetric table really does contain colorimetric data (and Argyll/dispcalGUI offer no choice in that matter), but which table contains what gamut mapping is not formally defined by the ICC spec, and profile generation software is pretty much allowed to generate whatever it sees fit.

Beep6581 commented 8 years ago
  1. I tried various output profile and rendering intent settings on a photo with colors that go out of gamut, and the Soft-proof button had no effect. Could you tell me a combination which works on your end, so I can try? I don't think it's necessary to store it in options, in fact I would prefer it not to.
  2. I'm confused, you say "it does nothing", but above you wrote that the "output color profile and intent is now shown with soft-proofing on only", so it should do something?
Hombre57 commented 8 years ago

Oups, sorry, I miss-read your first comment, I thought you were talking about the intent button that effectively does nothing when pressing on it.

I just selected the printer color profile in Output Profile + any intent + selecting a monitor profile (of course, otherwise the buttons should be insensitive), and activating soft-proofing made a big difference. I was also using wide gamut as WCS, but I don't think it makes a difference when soft-proofing a printer.

Hombre57 commented 8 years ago

I also need an answer to those questions:

As you can see, the Rendering intent is a graphical button for the monitor profile and a combobox for the Output rendering intent. I can do a graphical button too, but i guess that it have to stay sensitive even when using the manual output gamma. Should I add it below the Output profile combobox or on its right ?

And last but not least, would it be interesting to support lcms' own "black preservation" rendering intent ?

Beep6581 commented 8 years ago

I would say "to the right" only because I want us to save vertical screen space so you need to scroll less, otherwise no preference. Speaking of vertical screen space, now the DCP checkboxes are in a single column, instead of the previous vertical space-efficient 2x2 grid. Was that intentional?

About the LCMS "black preservation" intent, it sounds like its not relevant to RT, as the working space is never CMYK. http://www.littlecms.com/1/TUTORIAL.TXT This document states

Black preservation deals with CMYK -> CMYK transforms, and is intended to preserve, as much as possible, the black (K) channel whilst matching color by using CMY inks.

Hombre57 commented 8 years ago

@adamreichold Do you know if there's a way to figure out if the branch has been rebased before pulling ?

Beep6581 commented 8 years ago

@Hombre57 you asked me to increase the transparency of the Soft-proof button's icon, but I see it was designed correctly, with 70% opacity as is standard, so I don't think we need to change that.

Here are icon suggestions: http://filebin.net/glc1zctu6o/intent.zip I changed the "relative" icon to better represent that out-of-gamut colors will get clipped to the nearest in-gamut color. Included is also another set of perceptual, relative and saturation icons with a different approach - using the CIE xy chart. I think these look nicer (because they're smooth and round), but yours explain what's happening better. Maybe the ideal middle ground will be to use your approach with the lines, but just to make them smoother?

Hombre57 commented 8 years ago

@Beep6581 Your icons are nice, but I agree on the "self-explanatory" point. I'll make them smoother.

I'll add the Saturation entry for the monitor profile intent, but for the scroll wheel feature, I think it's gonna be complicated because the base object is a button, not a combobox and I don't want to loose too much time for this convenience.

So have you tried with a printer profile ? Is it working fine ?

Hombre57 commented 8 years ago

Speaking of vertical screen space, now the DCP checkboxes are in a single column, instead of the previous vertical space-efficient 2x2 grid. Was that intentional?

Yes, French strings are larger and I had to scroll horizontally with a reasonably wide right panel because the column's width are homogeneous (same width). Maybe other translation could be even worst.

adamreichold commented 8 years ago

@Hombre57 Short of trying out pull and aborting that, you could probably look at the difference log, e.g. do git fetch and then look at git log foo...origin/foo to see what a merge would probably do.

Basically, if you are uncomfortable with rebasing feature branches, we do not need to do that. It is definitely not an universally accepted point of view that commit history should be as clean and readable as possible, and some argue that the opposite is true, i.e. they should show everything that happened over time. Personally, I am firmly in the first camp, but the only effect of not rebasing the feature work will be harder to read code reviews and commit logs, but I think we could live with that.

Hombre57 commented 8 years ago

@adamreichold I'm not against rebasing, I just have to learn how to do that through Eclipse and avoid conflict when pulling.

Beep6581 commented 8 years ago

As I understand it git pull is fine to do when you just want the latest stuff and made no changes yourself, but if you did code something then its much safer to do git fetch instead, because git pull first does git fetch and then git merge into the branch you're currently on which could cause trouble.

Beep6581 commented 8 years ago

Rebasing explained, and when not to do it: https://www.youtube.com/watch?v=cSf8cO0WB4o