On 06/29/2015 06:09 PM, Derek Foreman wrote:
On 28/06/15 10:17 PM, Mario Kleiner wrote:
On 06/26/2015 08:29 PM, Derek Foreman wrote:
On 21/06/15 02:25 PM, Mario Kleiner wrote:
Allows to force loading an identity gamma table if
option icc_profile=identity is given in weston.ini for
an output.

Some special display output devices, e.g., for
neuro-science applications, and special display
testing hardware need a guaranteed perfect pixel
passthrough from framebuffer to output. This is
an easy way to set this up for cms-static.

v2: Remove confusing/redundant weston_log debug output.

Signed-off-by: Mario Kleiner <[email protected]>
---
   src/cms-static.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cms-static.c b/src/cms-static.c
index 7166f57..e6073df 100644
--- a/src/cms-static.c
+++ b/src/cms-static.c
@@ -56,7 +56,7 @@ cms_output_created(struct cms_static *cms, struct
weston_output *o)
       if (weston_config_section_get_string(s, "icc_profile",
&profile, NULL) < 0)
           return;
       p = weston_cms_load_profile(profile);
-    if (p == NULL) {
+    if (p == NULL && strcmp(profile, "identity")) {
           weston_log("cms-static: failed to load %s\n", profile);
       } else {
           weston_log("cms-static: loading %s for %s\n",


Can we use the empty string instead of "identity"?  Or is it intentional
that someone could override "identity" with a file of that name?


I think there isn't much danger of a mixup with the "identity" keyword,
as 'profile' otherwise needs to be a full "path/filename" to the color
profile file. Also these files usually end with a suffix like .icc

Ah, ok, so a user would have to have a profile called "identity"
(without the usual .icc extension) in the current directory for
something weird to happen.

I'll concede that this is rather unlikely.

Personally, I still don't like using a potentially valid filename like
this.  However, my say isn't final, so you're probably better off
waiting for someone else's review before making any changes. :)

Why isn't there just an identity.icc that can be loaded normally?


That could be done as well, just given that the code already implements the identity setup, although normally only triggered in case of an error, this seems like a convenient way to put that code to good use without the need to have an extra .icc file for this case.

The other thought i had is that currently the builtin identity code only sets an identity lut, but modern gpu's have more and more hw to do color transformations, digital display dithering, - or from the perspective of special neuroscience equipment or other special hardware testing equipment - more ways to mess up an identity pixel passthrough from framebuffer to video output. If we already have dedicated code builtin to get an identity mapping, one can extend that code as needed, whereas i don't know if everything can be covered by a custom loadable color profile.

I'm not opposed to changing it, but i'd find it a bit confusing to have
a config option with an empty assignment in the config file a la

icc_profile=

It would look like a configuration error in the config file to me?

We currently test for "no input method" in exactly this way (as of very
recently)

path=   means no input method
path being absent entirely means use the default (weston-keyboard)


If this is a common way to do it in westons config file, i can change the patch to accept the empty assignment as "set identity mapping", no problem.

Should we just load an identity gamma curve if there's no profile
specified anyway?  Or is there a situation where someone wants to mess
up the gamma curve before launching weston?


You could have a setup where the user wants to set a profile on some
output, e.g., "identity", for such outputs with special display
equipment connected, but leave it to system defaults on others. E.g., i
usually have weston with drm/kms backend running on one VT for testing,
but classic XOrg on another VT for development. It's kind of convenient
that XOrg will color-manage the outputs which don't get overriden by a
specific profile.

Interesting, I'd have considered X not reverting to the previous color
management state on a VT switch a bug. :)


Sometimes a bug can be a feature ;-). At least for me at the moment.

colord support in weston is not in such a robust shape atm. Without the last patch in my updated series you can't even use the cms-colord plugin without crashing weston at compositor shutdown most of the time - including a hard lockup of VT switching in most cases, so a reboot of the machine is needed. There are other ways left to crash it, e.g., if one tries to request updates of color profiles too fast. For some of my use cases i'd ideally need a way to set gamma tables quickly, so there's more work to do...

And if another DE runs under X in parallel on another VT, you have multiple instances of colord and some DBUS funkyness which makes it quite a tedious experience to try to test or use color management.

Is "system default" a meaningful concept here?  Or does it just mean
"whatever the last thing set it to"?

"whatever the last thing set it to" is what i meant a bit sloppy with "system defaults".

-mario
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to