On 07/02/2015 01:57 PM, Pekka Paalanen wrote:
On Thu, 02 Jul 2015 09:36:47 +0200
Mario Kleiner <[email protected]> wrote:
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.
Yeah, I'd prefer this too. Or, if we went with "identity", it would
need to be checked before any file loading is attempted, so that a
random file called "identity" would not even be attempted to be loaded,
even if it's not a valid icc profile.
Ok, just sent out v3 of the patch. Now loads the identity mapping if an
empty assignment "icc_profile=" is given.
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.
A bug indeed. When a display server takes over on VT switch, I'd expect
it either needs to reinitialize all display hardware state it intends
to manage or load the current setup from the driver, so it's up-to-date on
what is in the hardware.
Anything that relies on sloppiness there is bound to break when it gets
fixed.
Ok, just retested this again. X protects itself properly and apparently
reloads its own gamma tables when VT switching to it.
Weston otoh. doesn't restore its setting when switching to its VT. That
was the bug/"feature" that came in handy for me during initial testing
with my equipment, as i could use the parallel X-Session to set proper
gamma tables and then VT switch to Weston.
So, adding "identity" feature is nice, but then it should really
enforce identity at all times it is controlling the output.
I thought we already did reload the gamma tables on VT-switch in. Or at
least I have a vague recollection of it being discussed. Maybe backends
do it on their own, but don't call to CMS? Which means you'd get your
profile on first start, but VT-switch would reset it to identity or
something. Anyone seen such problems? Or was it just fbdev backend?
Currently only the drm_backend implements gamma table control by hooking
up the output->base.set_gamma method to drm_output_set_gamma. And that
method is only called from the cms code and not on VT switch.
-mario
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
