Discussion:
[patches] Add a color management framework to weston
(too old to reply)
Richard Hughes
2013-04-03 19:47:44 UTC
Permalink
I've attached three patches for comments and review. I'm a GLib
programmer at heart, so please be kind if I've made huge obvious
architectural mistakes :)

Patch 1 adds the framework code, patch 2 adds the ability to load CMS
modules, and patch 3 adds a CMS module written for colord. With those
three patches it actually works, and you can change the applied
profile at runtime using gnome-color-manager or colord-kde. Note, this
patchset just aims to set the calibration state, but I have left the
lcms hProfile handle available for when we want to construct 3d
shaders for the sub-surface color correction. I've also used all the
new features in colord 0.1.32, but say if you need/want me to tone
this down to something Ubuntu/whatever is shipping.

I've left a wodge of documentation in src/cms.h explaining all the
terms and the general idea on how I think this should work. There are
a ton of FIXMEs in relation to getting the EDID data, so ideas welcome
there too. Feedback very welcome, thanks.

Richard.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Add-a-colord-implementation-of-a-CMS-plugin-for-west.patch
Type: application/octet-stream
Size: 10157 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130403/817d31ae/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-the-ability-to-load-CMS-implementations-from-plu.patch
Type: application/octet-stream
Size: 1542 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130403/817d31ae/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-initial-color-management-framework-code.patch
Type: application/octet-stream
Size: 19074 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130403/817d31ae/attachment-0005.obj>
John Kåre Alsaker
2013-04-04 06:58:05 UTC
Permalink
The weston_color_manager struct could lose the state field. The CMS
plugins allocates it and can add state as needed. You should move the
weston_cms_output_added call below weston_output_init (which
initializes the compositor pointer in the outputs) and drop the first
argument from output_added, output_removed, weston_cms_output_added
and weston_cms_output_removed.

You should make a function which calls set_color_profile in weston
instead of calling it directly from the plugins. weston may want to do
some non-backend specific processing (such as informing the renderer
of the new profile). color_profile has to move from drm_output to
weston_output. The struct weston_color_profile * argument can then be
dropped from set_color_profile. Perhaps you should rename
set_color_profile to updated_color_profile. You should also move the
appropriate code from drm_output_set_color_profile into this function.
It should be called by the drm backend when using a hardcoded profile.
Also allow set_color_profile to be null, which is the case for most
backends.

I think struct weston_color_profile should also be created in this
function with filename and lcms_handle passed as arguments. The CMS
plugin should pass ownership of the lcms_handle to weston.

I don't see a GLib event loop, so I'm curious to how the signals work.
Post by Richard Hughes
I've attached three patches for comments and review. I'm a GLib
programmer at heart, so please be kind if I've made huge obvious
architectural mistakes :)
Patch 1 adds the framework code, patch 2 adds the ability to load CMS
modules, and patch 3 adds a CMS module written for colord. With those
three patches it actually works, and you can change the applied
profile at runtime using gnome-color-manager or colord-kde. Note, this
patchset just aims to set the calibration state, but I have left the
lcms hProfile handle available for when we want to construct 3d
shaders for the sub-surface color correction. I've also used all the
new features in colord 0.1.32, but say if you need/want me to tone
this down to something Ubuntu/whatever is shipping.
I've left a wodge of documentation in src/cms.h explaining all the
terms and the general idea on how I think this should work. There are
a ton of FIXMEs in relation to getting the EDID data, so ideas welcome
there too. Feedback very welcome, thanks.
Richard.
_______________________________________________
wayland-devel mailing list
wayland-devel at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Richard Hughes
2013-04-04 15:33:32 UTC
Permalink
The weston_color_manager struct could lose the state field....
All fixed in the newest patchset, thanks for your detailed review.
I don't see a GLib event loop, so I'm curious to how the signals work.
As discussed on IRC, I've used a thread for this. If wayland-glib can
do what we need in the future, I'll switch to using that if the dep is
okay.

New patches attached. Comments and further review welcomed as usual. Thanks.

Richard
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-initial-color-management-framework-code.patch
Type: application/octet-stream
Size: 20663 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130404/ce0a5e42/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-a-colord-implementation-of-a-CMS-plugin-for-west.patch
Type: application/octet-stream
Size: 10490 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130404/ce0a5e42/attachment-0003.obj>
John Kåre Alsaker
2013-04-04 16:11:32 UTC
Permalink
You can move the output->base.updated_color_profile assignment down to
the other of the vfunc initializations.

You should remove the destroy and user_data fields from weston_color_profile.

weston_cms_create_profile and weston_cms_load_profile should just
return a pointer to weston_color_profile.

Plugins should just exit if compositor->cms is already set in module_init.

The call to colord_update_output_from_device in colord_output_added
should probably be done in the GLib thread so it won't block the
compositor.

colord_update_output_from_device has to execute in the weston thread.
I suggest you create a wayland event source and plug that in to the
weston event loop. Make a thread safe list of pending output profile
updates. When a new update is added trigger the event source. The
event source handler should apply all the updates in the list. You
should also remove any pending updates for outputs in
colord_output_removed.
Post by Richard Hughes
The weston_color_manager struct could lose the state field....
All fixed in the newest patchset, thanks for your detailed review.
I don't see a GLib event loop, so I'm curious to how the signals work.
As discussed on IRC, I've used a thread for this. If wayland-glib can
do what we need in the future, I'll switch to using that if the dep is
okay.
New patches attached. Comments and further review welcomed as usual. Thanks.
Richard
Richard Hughes
2013-04-05 08:53:56 UTC
Permalink
Post by John KÃ¥re Alsaker
You should remove the destroy and user_data fields from weston_color_profile.
Fixed.
Post by John KÃ¥re Alsaker
weston_cms_create_profile and weston_cms_load_profile should just
return a pointer to weston_color_profile.
Fixed.
Post by John KÃ¥re Alsaker
Plugins should just exit if compositor->cms is already set in module_init.
Fixed.
Post by John KÃ¥re Alsaker
The call to colord_update_output_from_device in colord_output_added
should probably be done in the GLib thread so it won't block the
compositor.
I spent a few hours late last night looking at what could be run in
different threads, but two things became clear.

* GLib really wants to return all signals (colord_device_changed_cb)
on the main thread, not on the thread that's running the loop.
* Although the two _sync() calls look scary, I acted on a hunch and
added a timer to see how long getting the profile really took: 7ms

I think for the sake of not over-complicating things with *hundreds*
of lines of extra code, for an event that might only happen once or
twice per day or perhaps never at all, blocking the compositor for 7ms
is just fine. Changing the screen calibration isn't something that
happens frequently at all.

I've attached v3 of the patchset, I hope this is okay.

Richard.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-initial-color-management-framework-code.patch
Type: application/octet-stream
Size: 20416 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130405/b0e54426/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-a-colord-implementation-of-a-CMS-plugin-for-west.patch
Type: application/octet-stream
Size: 10755 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130405/b0e54426/attachment-0003.obj>
Richard Hughes
2013-04-05 16:45:08 UTC
Permalink
Post by Richard Hughes
* GLib really wants to return all signals (colord_device_changed_cb)
on the main thread, not on the thread that's running the loop.
As discussed on IRC, it seems I was wrong. I've attached both patches
(v6) for review. I've switched the coldplug onto the cms thread, and
switched to using wl_event_loop_add_fd() to batch up the updates.

I've got to move onto other stuff next week, but I'd be really
interested if anyone has inputs on how the sub-surface gamut mapping
using shaders is going to work. The main complication seems to be that
it's per-output, rather than per-surface. I've played with 1bit masks
in the past, but this made the shader quite complicated. Ideas very
welcome. Thanks.

Richard
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-initial-color-management-framework-code.patch
Type: application/octet-stream
Size: 20416 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130405/179bb146/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-a-colord-implementation-of-a-CMS-plugin-for-west.patch
Type: application/octet-stream
Size: 13988 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130405/179bb146/attachment-0003.obj>
Bill Spitzak
2013-04-05 18:14:02 UTC
Permalink
Post by Richard Hughes
I've got to move onto other stuff next week, but I'd be really
interested if anyone has inputs on how the sub-surface gamut mapping
using shaders is going to work. The main complication seems to be that
it's per-output, rather than per-surface. I've played with 1bit masks
in the past, but this made the shader quite complicated. Ideas very
welcome. Thanks.
The compositor should be able to draw N polygons for the N intersections
of a surface with each output, each using a different shader?

You absolutely do not want to put an if based on a pixel value into a
glsl shader. It is really slow.
Thomas Daede
2013-04-05 19:23:50 UTC
Permalink
I am not sure that doing the color conversion in the compositor is the
correct place. Some of it has to be there to support vcgt, but for
more general conversions, it gets complicated quickly.

Color correction is most important for artists doing work in something
like the GIMP. Programs like this (as of GIMP 3.0, at least) generally
work in higher bit depths - 16 bit, half float, or sometimes 32 bit
float. It's much better to do conversion in the higher bit depth, then
dither to the final display depth. Doing this with the compositor
involves supporting all of these texture formats for subsurfaces -
also, the toolkit has to support subsurfaces, because generally the UI
will be some lower bit depth, or not need correction.

Converting bit depths in the compositor would also require an extra
buffer allocation and copy in the compositor.

RGB images are also not the only thing that needs to be color
corrected - for example, 10bit YUV video streams might want to be
color corrected too. If we were to go as far as converting bit depths
in the compositor, it wouldn't be much to add this.

I think that providing color space regions to the client, relative to
the client's buffer, including vcgt settings, would shove a lot of
complexity away to clients that are already used to dealing with it.
The compositor should be able to draw N polygons for the N intersections of a surface with each output, each using a different shader?
You absolutely do not want to put an if based on a pixel value into a glsl shader. It is really slow.
This is the correct way, I think. Splitting surfaces by monitor also
has other unrelated benefits, like being able to sync separately to
each monitor - it probably should go somewhere else.
Richard Hughes
2013-04-05 20:06:22 UTC
Permalink
Post by Thomas Daede
Color correction is most important for artists doing work in something
like the GIMP. Programs like this (as of GIMP 3.0, at least) generally
work in higher bit depths
For GIMP I was going to just allow the app to opt-out a sub-surface
from any kind of color management. i.e. do it all in the app.
Post by Thomas Daede
The compositor should be able to draw N polygons for the N intersections of a surface with each output, each using a different shader?
You absolutely do not want to put an if based on a pixel value into a glsl shader. It is really slow.
This is the correct way, I think. Splitting surfaces by monitor also
has other unrelated benefits, like being able to sync separately to
each monitor - it probably should go somewhere else.
Right, I'll look into per-output shader code next week.

Richard
Bill Spitzak
2013-04-05 20:33:54 UTC
Permalink
Post by Richard Hughes
For GIMP I was going to just allow the app to opt-out a sub-surface
from any kind of color management. i.e. do it all in the app.
Maybe the client should be able to query the color space of the output
and can set the color space of it's buffer to the same one, this would
indicate that no color correction is needed. The compositor could still
color-correct it from one output's color space to another's if it is on
multiple outputs.

Ideally a client should be able to do this with just some strings (ie
query the color space and send the same string as the buffer's color
space) so it does not have to open a CMS library that it might otherwise
not be using.
Graeme Gill
2013-04-05 22:41:26 UTC
Permalink
Post by Bill Spitzak
Maybe the client should be able to query the color space of the output
and can set the color space of it's buffer to the same one, this would
indicate that no color correction is needed. The compositor could still
color-correct it from one output's color space to another's if it is on
multiple outputs.
This is not as simple to set and as safe as an explicit opt out.
(Apple has had a lot of trouble as a result of doing their
color management opt out this way). It's hard to confirm that you've got
the same profile ("same" in what sense ? Same instance of the profile ?
Same bits ? Same notional colorspace ?)
You would need a special case anyway to implement a null transform,
because things like cLUT profiles have A2B and B2A tables that are not
perfect inverses.

You'd also want to be sure that a null transform is bit perfect, and
that something isn't lost in shaders etc.

If this idea is still attractve, I would suggest supporting device links,
making it reasonably simple to set a null device link.
Post by Bill Spitzak
Ideally a client should be able to do this with just some strings (ie
query the color space and send the same string as the buffer's color
space) so it does not have to open a CMS library that it might otherwise
not be using.
If that's the case, I'd suggest having a string that turns the color management
off. Much simpler and more direct as to the intention.

Graeme Gill.
Graeme Gill
2013-04-05 22:31:30 UTC
Permalink
Post by Thomas Daede
I am not sure that doing the color conversion in the compositor is the
correct place. Some of it has to be there to support vcgt, but for
more general conversions, it gets complicated quickly.
Hi,

The expectation is that vcgt is per output (ie., it's loaded into
the display cards RAMDAC), or loaded into the display itself
using DDC. The display device color profile assumes this.
So vcgt shouldn't need shaders.
Post by Thomas Daede
Color correction is most important for artists doing work in something
like the GIMP. Programs like this (as of GIMP 3.0, at least) generally
As well as this, there is a desire to color manage all GUI elements
on a wide gamut display, to avoid a rather garish and hard to read
result.
Post by Thomas Daede
I think that providing color space regions to the client, relative to
the client's buffer, including vcgt settings, would shove a lot of
complexity away to clients that are already used to dealing with it.
One of the things driving the idea of tagging a colorspace and
letting the display system handle color management by default, is
that many/most applications don't deal with it. So I think that the
idea of shoving off the learning curve about color to a larger group of
applcation writers is not likely to be successful.
Post by Thomas Daede
This is the correct way, I think. Splitting surfaces by monitor also
has other unrelated benefits, like being able to sync separately to
each monitor - it probably should go somewhere else.
Application writers particularly do not like dealing with the
messiness of color managing their graphics elements when they
happen to fall across more than one output. A number of applications
get color management right on a single display, and fail badly with
more than one display.

Graeme Gill.
Pekka Paalanen
2013-04-22 10:30:28 UTC
Permalink
On Fri, 5 Apr 2013 14:23:50 -0500
Post by Thomas Daede
I am not sure that doing the color conversion in the compositor is the
correct place. Some of it has to be there to support vcgt, but for
more general conversions, it gets complicated quickly.
Color correction is most important for artists doing work in something
like the GIMP. Programs like this (as of GIMP 3.0, at least) generally
work in higher bit depths - 16 bit, half float, or sometimes 32 bit
float. It's much better to do conversion in the higher bit depth, then
dither to the final display depth. Doing this with the compositor
involves supporting all of these texture formats for subsurfaces -
also, the toolkit has to support subsurfaces, because generally the UI
will be some lower bit depth, or not need correction.
Converting bit depths in the compositor would also require an extra
buffer allocation and copy in the compositor.
RGB images are also not the only thing that needs to be color
corrected - for example, 10bit YUV video streams might want to be
color corrected too. If we were to go as far as converting bit depths
in the compositor, it wouldn't be much to add this.
I think that providing color space regions to the client, relative to
the client's buffer, including vcgt settings, would shove a lot of
complexity away to clients that are already used to dealing with it.
I'm not sure we can do that. The regions can be arbitrarily complex and
non-linear shapes.

I also do not believe that solving the color correctness problem for
a single surface spanning more than one monitor is worth all the pain
and special cases, unless the color correction is done in the
compositor.
Post by Thomas Daede
Post by Bill Spitzak
The compositor should be able to draw N polygons for the N
intersections of a surface with each output, each using a different
shader? You absolutely do not want to put an if based on a pixel
value into a glsl shader. It is really slow.
This is the correct way, I think. Splitting surfaces by monitor also
has other unrelated benefits, like being able to sync separately to
each monitor - it probably should go somewhere else.
Weston already does it like that. Each output is a framebuffer, the
desktop as a whole is not.


Thanks,
pq
Kristian Høgsberg
2013-05-01 21:03:30 UTC
Permalink
Post by Pekka Paalanen
On Fri, 5 Apr 2013 14:23:50 -0500
Post by Thomas Daede
I am not sure that doing the color conversion in the compositor is the
correct place. Some of it has to be there to support vcgt, but for
more general conversions, it gets complicated quickly.
Color correction is most important for artists doing work in something
like the GIMP. Programs like this (as of GIMP 3.0, at least) generally
work in higher bit depths - 16 bit, half float, or sometimes 32 bit
float. It's much better to do conversion in the higher bit depth, then
dither to the final display depth. Doing this with the compositor
involves supporting all of these texture formats for subsurfaces -
also, the toolkit has to support subsurfaces, because generally the UI
will be some lower bit depth, or not need correction.
Converting bit depths in the compositor would also require an extra
buffer allocation and copy in the compositor.
RGB images are also not the only thing that needs to be color
corrected - for example, 10bit YUV video streams might want to be
color corrected too. If we were to go as far as converting bit depths
in the compositor, it wouldn't be much to add this.
I think that providing color space regions to the client, relative to
the client's buffer, including vcgt settings, would shove a lot of
complexity away to clients that are already used to dealing with it.
I'm not sure we can do that. The regions can be arbitrarily complex and
non-linear shapes.
It's not too different from how we handle opaque regions. We split up
the surfaces into opaque and transparent parts and render them using
different shaders already.

Kristian
Pekka Paalanen
2013-05-02 06:58:48 UTC
Permalink
On Wed, 1 May 2013 17:03:30 -0400
Post by Kristian Høgsberg
Post by Pekka Paalanen
On Fri, 5 Apr 2013 14:23:50 -0500
Post by Thomas Daede
I am not sure that doing the color conversion in the compositor
is the correct place. Some of it has to be there to support vcgt,
but for more general conversions, it gets complicated quickly.
Color correction is most important for artists doing work in
something like the GIMP. Programs like this (as of GIMP 3.0, at
least) generally work in higher bit depths - 16 bit, half float,
or sometimes 32 bit float. It's much better to do conversion in
the higher bit depth, then dither to the final display depth.
Doing this with the compositor involves supporting all of these
texture formats for subsurfaces - also, the toolkit has to
support subsurfaces, because generally the UI will be some lower
bit depth, or not need correction.
Converting bit depths in the compositor would also require an
extra buffer allocation and copy in the compositor.
RGB images are also not the only thing that needs to be color
corrected - for example, 10bit YUV video streams might want to be
color corrected too. If we were to go as far as converting bit
depths in the compositor, it wouldn't be much to add this.
I think that providing color space regions to the client,
relative to the client's buffer, including vcgt settings, would
shove a lot of complexity away to clients that are already used
to dealing with it.
I'm not sure we can do that. The regions can be arbitrarily complex
and non-linear shapes.
It's not too different from how we handle opaque regions. We split up
the surfaces into opaque and transparent parts and render them using
different shaders already.
Sorry, I read "providing color space regions to the client" as "the
server determines the areas of each output overlapping with the
surface", and communicated that to the clients.

If the regions are communicated from clients to the server direction
only, then nevermind my comment.


Thanks,
pq
John Kåre Alsaker
2013-05-02 12:21:29 UTC
Permalink
I'd suggest that client should use subsurfaces if they want multiple
colorspaces in a window. They might have to do that anyway since they may
want a different pixel format.
Post by Pekka Paalanen
On Wed, 1 May 2013 17:03:30 -0400
Post by Kristian Høgsberg
Post by Pekka Paalanen
On Fri, 5 Apr 2013 14:23:50 -0500
Post by Thomas Daede
I am not sure that doing the color conversion in the compositor
is the correct place. Some of it has to be there to support vcgt,
but for more general conversions, it gets complicated quickly.
Color correction is most important for artists doing work in
something like the GIMP. Programs like this (as of GIMP 3.0, at
least) generally work in higher bit depths - 16 bit, half float,
or sometimes 32 bit float. It's much better to do conversion in
the higher bit depth, then dither to the final display depth.
Doing this with the compositor involves supporting all of these
texture formats for subsurfaces - also, the toolkit has to
support subsurfaces, because generally the UI will be some lower
bit depth, or not need correction.
Converting bit depths in the compositor would also require an
extra buffer allocation and copy in the compositor.
RGB images are also not the only thing that needs to be color
corrected - for example, 10bit YUV video streams might want to be
color corrected too. If we were to go as far as converting bit
depths in the compositor, it wouldn't be much to add this.
I think that providing color space regions to the client,
relative to the client's buffer, including vcgt settings, would
shove a lot of complexity away to clients that are already used
to dealing with it.
I'm not sure we can do that. The regions can be arbitrarily complex
and non-linear shapes.
It's not too different from how we handle opaque regions. We split up
the surfaces into opaque and transparent parts and render them using
different shaders already.
Sorry, I read "providing color space regions to the client" as "the
server determines the areas of each output overlapping with the
surface", and communicated that to the clients.
If the regions are communicated from clients to the server direction
only, then nevermind my comment.
Thanks,
pq
_______________________________________________
wayland-devel mailing list
wayland-devel at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130502/39b7676e/attachment.html>
Matthias Clasen
2013-04-04 20:30:03 UTC
Permalink
On Thu, Apr 4, 2013 at 2:58 AM, John K?re Alsaker
Post by John KÃ¥re Alsaker
I don't see a GLib event loop, so I'm curious to how the signals work.
Just ftr (and without actually looking at the patch): glib signals
don't need an event loop, they're entirely synchronous.
Richard Hughes
2013-04-05 08:27:18 UTC
Permalink
Post by Matthias Clasen
Just ftr (and without actually looking at the patch): glib signals
don't need an event loop, they're entirely synchronous.
Right, but in this case the event is coming from the GDBus thread,
which I assume is g_idle_add'ing the signal it back to the mainloop so
that it's executed in the main thread.

Richard.
Bill Spitzak
2013-04-04 18:08:09 UTC
Permalink
How is this intended to work in Wayland?

Are clients supposed to tell the compositor what color space their
buffer is, and the compositor then converts to the output's color space
as part of the compositing step? Problems with this:

* We need a reliable way for clients to tell compositor what color
space they are rendering, ideally without huge amounts of code such as
the same color space converter library in the client.
* Fast conversion of an 8-bit buffer between color spaces reduces the
effective number of bits of color to 7 or even 6, and can introduce ugly
artifacts. Adding enough noise (what most shaders call dithering) can
hide this but obviously adds noise. Computing the output color space at
higher accuracy and then doing proper error diffusion dithering is
better, but slow on many GPUs. Requiring the buffers to be higher than
8-bit precision can fix this but people may question Wayland's
commitment to allowing efficient clients.
* Is "same as the output" a supported color space?

Or are clients supposed to get the color space needed and convert to it
when rendering their buffers? This seems more Wayland-like but has problems:

* problem if the surface is on more than one output
* multiple copies of the color-correction library and loaded color
space descriptions in each client
* need for message from compositor to client to change the color
space, and compositor must wait for commit from all clients before
updating the screen.
* Clients may not have access to the color correction hardware that
the compositor has.
Post by Richard Hughes
I've attached three patches for comments and review. I'm a GLib
programmer at heart, so please be kind if I've made huge obvious
architectural mistakes :)
Richard Hughes
2013-04-04 18:39:03 UTC
Permalink
Are clients supposed to tell the compositor what color space their buffer
is, and the compositor then converts to the output's color space as part of
the compositing step?
At the moment I'm just implementing the calibration state setting,
i.e. setting the gamma ramps from the VCGT data. When we have
sub-surfaces to play with I was going to look at tagging subsurfaces
with different color spaces and setting up shaders to do the color
conversion to display space, but for the moment I was trying to keep
it simple.

Richard.
Bill Spitzak
2013-04-04 18:52:11 UTC
Permalink
Post by Richard Hughes
Are clients supposed to tell the compositor what color space their buffer
is, and the compositor then converts to the output's color space as part of
the compositing step?
At the moment I'm just implementing the calibration state setting,
i.e. setting the gamma ramps from the VCGT data. When we have
sub-surfaces to play with I was going to look at tagging subsurfaces
with different color spaces and setting up shaders to do the color
conversion to display space, but for the moment I was trying to keep
it simple.
Okay, it does sound like the compositor is doing the conversion.

These gamma ramps are an attempt to simulate a color conversion from
some color space (sRGB?) to the color space of the output device. So
does this mean that the default color space of the buffers a client
renders is sRGB?
Richard Hughes
2013-04-04 19:03:37 UTC
Permalink
These gamma ramps are an attempt to simulate a color conversion from some
color space (sRGB?) to the color space of the output device.
Not really, the gamma ramps just affect the gamma response and the
display whitepoint. You can't do color gamut mapping as the curves are
1D. Apple introduced the vcgt calibration tag, and it's been used for
a few years mainly for changing the whitepoint.

Richard
Graeme Gill
2013-04-05 10:52:55 UTC
Permalink
Post by Bill Spitzak
These gamma ramps are an attempt to simulate a color conversion from
some color space (sRGB?) to the color space of the output device.
No, they are not. Gamma ramp setting generally accomplishes one or
more of:

1) Setting the brightness to a calibrated level.
2) Setting the white point to a target color temperature.
3) Setting the gray response to a particular curve and
neutrality.

Although you might be able to accomplish 2) using ICC profiles,
it isn't supported very well (absolute colorimetric & V4 ICC profiles etc.),
and it's better for the users state of adaptation if the whole screen
has a common white point.

3) helps non-color managed application, as well as making the
display more easily and accurately color profile-able.
Post by Bill Spitzak
So > does this mean that the default color space of the buffers a client
renders is sRGB?
You can't do that with 1D ramps - you need at least a matrix as well.
For general colorspace emulation you need a 3D cLUT.

Graeme Gill.
Kai-Uwe Behrmann
2013-04-04 18:21:41 UTC
Permalink
Post by Richard Hughes
Patch 1 adds the framework code, patch 2 adds the ability to load CMS
modules, and patch 3 adds a CMS module written for colord. With those
People have a hard time to understand ICC and colour termininology. So
some annotations to that first.
A CMS module for a CMF. That is in conflict with your explanations. You
defined colord as a CMF but call it a CMS module (one which can do by
your definition colour conversions like ColorSync, WCS or Oyranos). But
the colord API can not do any colour conversion as would be typical for
a CMS.

You weston code is outlined as a CMS not a CMF (in your terminology),
because you link against a CMM.

In case you like to provide the device configuration side only, then
lcms is not needed at all.

In case colour conversion is part of the plan for weston, then calling
this code a CMS is spot on.

To the actual code,
Parsing of ICC profiles inside weston is not nice. That would be better
abstracted out to lay inside the CMS plugins. Those CMS plugins can link
against lcms or whatever CMM they prefere.
Post by Richard Hughes
I've left a wodge of documentation in src/cms.h explaining all the
terms and the general idea on how I think this should work. There are
You explain the term "CMF". The first idea, which pops to my mind, is
the "Colour Matching Function", which is fundamental to the CIE and ICC
specs. Are you aiming to do spectral imaging in weston?

kind regards
Kai-Uwe
Richard Hughes
2013-04-04 18:34:40 UTC
Permalink
Post by Kai-Uwe Behrmann
Parsing of ICC profiles inside weston is not nice. That would be better
abstracted out to lay inside the CMS plugins. Those CMS plugins can link
against lcms or whatever CMM they prefere.
Well, we could certainly add some abstraction to get 3 allocated gamma
ramps, although I don't think we should aim to abstract everything
without a good reason. It's not likely a different cms will want to
load the vcgt calibration data in a different way after all. It's
basically table data.
Post by Kai-Uwe Behrmann
Are you aiming to do spectral imaging in weston?
No, that would be silly.

Richard.
Continue reading on narkive:
Loading...