Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sway/commands/output: Add command to set color profile #7681

Merged
merged 1 commit into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .builds/alpine.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ packages:
- eudev-dev
- gdk-pixbuf-dev
- json-c-dev
- lcms2-dev
- libdisplay-info-dev
- libevdev-dev
- libinput-dev
Expand Down
1 change: 1 addition & 0 deletions .builds/archlinux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ packages:
- cairo
- gdk-pixbuf2
- json-c
- lcms2
- libdisplay-info
- libegl
- libinput
Expand Down
1 change: 1 addition & 0 deletions .builds/freebsd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ packages:
- devel/pkgconf
- graphics/cairo
- graphics/gdk-pixbuf2
- graphics/lcms2
- graphics/wayland
- graphics/wayland-protocols
- textproc/scdoc
Expand Down
1 change: 1 addition & 0 deletions include/sway/commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ sway_cmd input_cmd_xkb_variant;

sway_cmd output_cmd_adaptive_sync;
sway_cmd output_cmd_background;
sway_cmd output_cmd_color_profile;
sway_cmd output_cmd_disable;
sway_cmd output_cmd_dpms;
sway_cmd output_cmd_enable;
Expand Down
3 changes: 3 additions & 0 deletions include/sway/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <wlr/interfaces/wlr_switch.h>
#include <wlr/types/wlr_tablet_tool.h>
#include <wlr/util/box.h>
#include <wlr/render/color.h>
#include <xkbcommon/xkbcommon.h>
#include <xf86drmMode.h>
#include "../include/config.h"
Expand Down Expand Up @@ -285,6 +286,8 @@ struct output_config {
int max_render_time; // In milliseconds
int adaptive_sync;
enum render_bit_depth render_bit_depth;
bool set_color_transform;
struct wlr_color_transform *color_transform;

char *background;
char *background_option;
Expand Down
2 changes: 2 additions & 0 deletions include/sway/output.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ struct sway_output {
struct wl_signal disable;
} events;

struct wlr_color_transform *color_transform;

struct timespec last_presentation;
uint32_t refresh_nsec;
int max_render_time; // In milliseconds
Expand Down
1 change: 1 addition & 0 deletions sway/commands/output.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ static const struct cmd_handler output_handlers[] = {
{ "adaptive_sync", output_cmd_adaptive_sync },
{ "background", output_cmd_background },
{ "bg", output_cmd_background },
{ "color_profile", output_cmd_color_profile },
{ "disable", output_cmd_disable },
{ "dpms", output_cmd_dpms },
{ "enable", output_cmd_enable },
Expand Down
101 changes: 101 additions & 0 deletions sway/commands/output/color_profile.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#include <fcntl.h>
#include <strings.h>
#include <sys/stat.h>
#include <unistd.h>
#include <wlr/render/color.h>
#include "sway/commands.h"
#include "sway/config.h"

static bool read_file_into_buf(const char *path, void **buf, size_t *size) {
/* Why not use fopen/fread directly? glibc will succesfully open directories,
* not just files, and supports seeking on them. Instead, we directly
* work with file descriptors and use the more consistent open/fstat/read. */
int fd = open(path, O_RDONLY | O_NOCTTY | O_CLOEXEC);
if (fd == -1) {
return false;
}
char *b = NULL;
struct stat info;
if (fstat(fd, &info) == -1) {
goto fail;
}
// only regular files, to avoid issues with e.g. opening pipes
if (!S_ISREG(info.st_mode)) {
goto fail;
}
off_t s = info.st_size;
if (s <= 0) {
goto fail;
}
b = calloc(1, s);
if (!b) {
goto fail;
}
size_t nread = 0;
while (nread < (size_t)s) {
size_t to_read = (size_t)s - nread;
ssize_t r = read(fd, b + nread, to_read);
if ((r == -1 && errno != EINTR) || r == 0) {
goto fail;
}
nread += (size_t)r;
}
close(fd);
*buf = b;
*size = (size_t)s;
return true; // success
mstoeckl marked this conversation as resolved.
Show resolved Hide resolved
fail:
free(b);
close(fd);
return false;
}

struct cmd_results *output_cmd_color_profile(int argc, char **argv) {
if (!config->handler_context.output_config) {
return cmd_results_new(CMD_FAILURE, "Missing output config");
}
if (!argc) {
return cmd_results_new(CMD_INVALID, "Missing color_profile first argument.");
}

if (strcmp(*argv, "srgb") == 0) {
wlr_color_transform_unref(config->handler_context.output_config->color_transform);
config->handler_context.output_config->color_transform = NULL;
config->handler_context.output_config->set_color_transform = true;

config->handler_context.leftovers.argc = argc - 1;
config->handler_context.leftovers.argv = argv + 1;
} else if (strcmp(*argv, "icc") == 0) {
if (argc < 2) {
return cmd_results_new(CMD_INVALID,
"Invalid color profile specification: icc type requires a file");
}
void *data = NULL;
size_t size = 0;
if (!read_file_into_buf(argv[1], &data, &size)) {
return cmd_results_new(CMD_FAILURE,
"Failed to load color profile: could not read ICC file");
}

struct wlr_color_transform *tmp =
wlr_color_transform_init_linear_to_icc(data, size);
if (!tmp) {
free(data);
return cmd_results_new(CMD_FAILURE,
"Failed to load color profile: failed to initialize transform from ICC");
}
free(data);

wlr_color_transform_unref(config->handler_context.output_config->color_transform);
config->handler_context.output_config->color_transform = tmp;
config->handler_context.output_config->set_color_transform = true;

config->handler_context.leftovers.argc = argc - 2;
config->handler_context.leftovers.argv = argv + 2;
} else {
return cmd_results_new(CMD_INVALID,
"Invalid color profile specification: first argument should be icc|srgb");
}

return NULL;
}
18 changes: 18 additions & 0 deletions sway/config/output.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ struct output_config *new_output_config(const char *name) {
oc->max_render_time = -1;
oc->adaptive_sync = -1;
oc->render_bit_depth = RENDER_BIT_DEPTH_DEFAULT;
oc->set_color_transform = false;
oc->color_transform = NULL;
oc->power = -1;
return oc;
}
Expand Down Expand Up @@ -191,6 +193,14 @@ static void merge_output_config(struct output_config *dst, struct output_config
if (src->render_bit_depth != RENDER_BIT_DEPTH_DEFAULT) {
dst->render_bit_depth = src->render_bit_depth;
}
if (src->set_color_transform) {
if (src->color_transform) {
wlr_color_transform_ref(src->color_transform);
}
wlr_color_transform_unref(dst->color_transform);
dst->set_color_transform = true;
dst->color_transform = src->color_transform;
}
if (src->background) {
free(dst->background);
dst->background = strdup(src->background);
Expand Down Expand Up @@ -557,6 +567,13 @@ static bool finalize_output_config(struct output_config *oc, struct sway_output
output->max_render_time = oc->max_render_time;
}

if (oc && oc->set_color_transform) {
if (oc->color_transform) {
wlr_color_transform_ref(oc->color_transform);
}
wlr_color_transform_unref(output->color_transform);
output->color_transform = oc->color_transform;
}
return true;
}

Expand Down Expand Up @@ -997,6 +1014,7 @@ void free_output_config(struct output_config *oc) {
free(oc->name);
free(oc->background);
free(oc->background_option);
wlr_color_transform_unref(oc->color_transform);
free(oc);
}

Expand Down
5 changes: 4 additions & 1 deletion sway/desktop/output.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,10 @@ static int output_repaint_timer_handler(void *data) {
return 0;
}

wlr_scene_output_commit(output->scene_output, NULL);
struct wlr_scene_output_state_options opts = {
.color_transform = output->color_transform,
};
wlr_scene_output_commit(output->scene_output, &opts);
return 0;
}

Expand Down
1 change: 1 addition & 0 deletions sway/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ sway_sources = files(
'commands/output/toggle.c',
'commands/output/transform.c',
'commands/output/unplug.c',
'commands/output/color_profile.c',

'tree/arrange.c',
'tree/container.c',
Expand Down
12 changes: 12 additions & 0 deletions sway/sway-output.5.scd
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,18 @@ must be separated by one space. For example:
updated to work with different bit depths. This command is experimental,
and may be removed or changed in the future.

*output* <name> color_profile srgb|[icc <file>]
Sets the color profile for an output. The default is _srgb_. <file> should be a
path to a display ICC profile.
Comment on lines +182 to +183
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this interact with HDR? I would assume that ICC is incompatible with HDR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know; I made this PR mainly to test the wlroots MR, and do not know what a good command interface for the long term would look like. Marking this PR as draft until this gets figured out; maybe the answer will be that, like render_bit_depth, this is a short-term approach to be replaced later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a warning that setting a color profile may have odd effects in conjunction with HDR.

Color profiles probably still have a meaningful interpretation even when when the HDR metadata is being sent to a display -- I believe the main difference is that the monitor handles pixel data differently in HDR mode, using absolute luminance, and uses PQ EOTF instead of approximate-sRGB. For example, it should still be possible to use color profiles to correct for monitor miscalibration; or just to apply general color transforms (e.g.: adjusting contrast, making everything brighter, rendering in grayscale, applying hue shifts -- although it might be cleaner to have such color changes be handled with a separate command, using 'Abstract' instead of 'Display' ICC profiles.) Of course, due to the difffering EOTFs being applied by the monitor, any profile used for SDR would produce invalid output for HDR.


Not all renderers support this feature; currently it only works with the
the Vulkan renderer. Even where supported, the application of the color
profile may be inaccurate.

This command is experimental, and may be removed or changed in the future. It
may have no effect or produce unexpected output when used together with future
HDR support features.

# SEE ALSO

*sway*(5) *sway-input*(5)
1 change: 1 addition & 0 deletions sway/tree/output.c
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ void output_destroy(struct sway_output *output) {
list_free(output->workspaces);
list_free(output->current.workspaces);
wl_event_source_remove(output->repaint_timer);
wlr_color_transform_unref(output->color_transform);
free(output);
}

Expand Down