From f3badf2c2db729d4f447c0c4439a1229934aef07 Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Tue, 17 Jun 2025 13:29:36 +0300 Subject: [PATCH] color: introduce struct weston_color_tf At first I wanted to wrap float tf_params[MAX_PARAMS_TF] into a struct, so that it would become type-safe to pass into functions, and it could be copied with a simple assignment. Then I noticed that for tf_params to be operable, it must always be accompanied by struct weston_color_tf_info. Hence, struct weston_color_tf was born. The need for #define MAX_PARAMS_TF got eliminated. Because struct weston_color_tf is a member of struct weston_color_profile_params, now both need to not contain implicit padding. This patch makes the internal enumerated TF structures follow the current protocol requests: all color channels use the same curve. Signed-off-by: Pekka Paalanen --- libweston/color-lcms/color-profile.c | 10 ++- libweston/color-lcms/color-transform.c | 86 +++++++++++-------- libweston/color-operations.c | 2 +- libweston/color-profile-param-builder.c | 12 +-- libweston/color.c | 62 ++++++------- libweston/color.h | 33 +++---- .../gl-shader-config-color-transformation.c | 2 +- 7 files changed, 110 insertions(+), 97 deletions(-) diff --git a/libweston/color-lcms/color-profile.c b/libweston/color-lcms/color-profile.c index d49bfd420..9a10952c9 100644 --- a/libweston/color-lcms/color-profile.c +++ b/libweston/color-lcms/color-profile.c @@ -383,11 +383,15 @@ cmlcms_find_color_profile_by_params(const struct weston_color_manager_lcms *cm, struct cmlcms_color_profile *cprof; /* Ensure no uninitialized data inside struct to make memcmp work. */ + static_assert(sizeof(params->tf) == + sizeof(params->tf.info) + + sizeof(params->tf.params) + + sizeof(params->tf.padding), + "struct weston_color_transfer_function must not contain implicit padding"); static_assert(sizeof(*params) == 2 * sizeof(float) * 2 * 4 + /* primaries, target_primaries */ sizeof(params->primaries_info) + - sizeof(params->tf_info) + - sizeof(params->tf_params) + + sizeof(params->tf) + sizeof(params->min_luminance) + sizeof(params->max_luminance) + sizeof(params->reference_white_luminance) + @@ -708,7 +712,7 @@ cmlcms_get_color_profile_from_params(struct weston_color_manager *cm_base, str_printf(&desc, "Parametric (%s): %s primaries, %s transfer function", name_part, params->primaries_info ? params->primaries_info->desc : "custom", - params->tf_info->desc); + params->tf.info->desc); cprof = cmlcms_color_profile_alloc(cm, CMLCMS_PROFILE_TYPE_PARAMS, desc); *cprof->params = *params; diff --git a/libweston/color-lcms/color-transform.c b/libweston/color-lcms/color-transform.c index 37912e714..ed301599d 100644 --- a/libweston/color-lcms/color-transform.c +++ b/libweston/color-lcms/color-transform.c @@ -450,20 +450,26 @@ init_curve_from_type_1(struct weston_compositor *compositor, tf_info = lcms_curve_matches_any_tf(compositor, 1, clamped_input, type_1_params); if (tf_info) { curve->type = WESTON_COLOR_CURVE_TYPE_ENUM; - enumerated->tf = tf_info; + enumerated->tf = (struct weston_color_tf){ + .info = tf_info, + .params = {} + }; enumerated->tf_direction = WESTON_FORWARD_TF; return true; } - /* This is a pure power-law with custom exp. If clamped_input == false, - * this matches WESTON_TF_POWER (parametric TF that is not clamped). */ - if (!clamped_input) { + /* This is a pure power-law with custom exp. If clamped_input == false + * and all channels behave the same, this matches WESTON_TF_POWER. */ + if (!clamped_input && + type_1_params[0][0] == type_1_params[1][0] && + type_1_params[0][0] == type_1_params[2][0]) { + tf_info = weston_color_tf_info_from(compositor, WESTON_TF_POWER); curve->type = WESTON_COLOR_CURVE_TYPE_ENUM; - enumerated->tf = weston_color_tf_info_from(compositor, - WESTON_TF_POWER); + enumerated->tf = (struct weston_color_tf){ + .info = tf_info, + .params = { type_1_params[0][0] } + }; enumerated->tf_direction = WESTON_FORWARD_TF; - for (i = 0; i < 3; i++) - enumerated->params[i][0] = type_1_params[i][0]; return true; } @@ -519,29 +525,34 @@ init_curve_from_type_1_inverse(struct weston_compositor *compositor, tf_info = lcms_curve_matches_any_tf(compositor, 1, clamped_input, type_1_params); if (tf_info) { curve->type = WESTON_COLOR_CURVE_TYPE_ENUM; - enumerated->tf = tf_info; + enumerated->tf = (struct weston_color_tf){ + .info = tf_info, + .params = {} + }; enumerated->tf_direction = WESTON_INVERSE_TF; return true; } /* This is the inverse of a pure power-law with custom exp. If - * clamped_input == false, this matches WESTON_TF_POWER (parametric TF - * that is not clamped). */ - if (!clamped_input) { - curve->type = WESTON_COLOR_CURVE_TYPE_ENUM; - enumerated->tf = weston_color_tf_info_from(compositor, - WESTON_TF_POWER); - enumerated->tf_direction = WESTON_INVERSE_TF; - for (i = 0; i < 3; i++) { - g = type_1_params[i][0]; - if (g == 0.0f) { - err_msg = "WARNING: xform has a LittleCMS type -1 curve " \ - "(inverse of pure power-law) with exponent 1 " \ - "divided by 0, which is invalid"; - goto err; - } - enumerated->params[i][0] = g; + * clamped_input == false and all channels behave the same, + * this matches WESTON_TF_POWER. */ + if (!clamped_input && + type_1_params[0][0] == type_1_params[1][0] && + type_1_params[0][0] == type_1_params[2][0]) { + if (type_1_params[0][0] == 0.0f) { + err_msg = "WARNING: xform has a LittleCMS type -1 curve " \ + "(inverse of pure power-law) with exponent 1 " \ + "divided by 0, which is invalid"; + goto err; } + + tf_info = weston_color_tf_info_from(compositor, WESTON_TF_POWER); + curve->type = WESTON_COLOR_CURVE_TYPE_ENUM; + enumerated->tf = (struct weston_color_tf){ + .info = tf_info, + .params = { type_1_params[0][0] } + }; + enumerated->tf_direction = WESTON_INVERSE_TF; return true; } @@ -615,7 +626,10 @@ init_curve_from_type_4(struct weston_compositor *compositor, tf_info = lcms_curve_matches_any_tf(compositor, 4, clamped_input, type_4_params); if (tf_info) { curve->type = WESTON_COLOR_CURVE_TYPE_ENUM; - enumerated->tf = tf_info; + enumerated->tf = (struct weston_color_tf){ + .info = tf_info, + .params = {} + }; enumerated->tf_direction = WESTON_FORWARD_TF; return true; } @@ -695,7 +709,10 @@ init_curve_from_type_4_inverse(struct weston_compositor *compositor, tf_info = lcms_curve_matches_any_tf(compositor, 4, clamped_input, type_4_params); if (tf_info) { curve->type = WESTON_COLOR_CURVE_TYPE_ENUM; - enumerated->tf = tf_info; + enumerated->tf = (struct weston_color_tf){ + .info = tf_info, + .params = {} + }; enumerated->tf_direction = WESTON_INVERSE_TF; return true; } @@ -1370,14 +1387,9 @@ weston_color_curve_set_from_params(struct weston_color_curve *curve, const struct weston_color_profile_params *p, enum weston_tf_direction dir) { - unsigned i; - curve->type = WESTON_COLOR_CURVE_TYPE_ENUM; - curve->u.enumerated.tf = p->tf_info; + curve->u.enumerated.tf = p->tf; curve->u.enumerated.tf_direction = dir; - - for (i = 0; i < 3; i++) - ARRAY_COPY(curve->u.enumerated.params[i], p->tf_params); } static void @@ -1961,9 +1973,9 @@ cmclms_adjust_recipe(struct cmlcms_color_transform_recipe *adjusted, * to be targeting a display with the sRGB two-piece TF is likely mistaken. */ if (in_prof->type == CMLCMS_PROFILE_TYPE_PARAMS && - in_prof->params->tf_info->tf == WESTON_TF_SRGB) { + in_prof->params->tf.info->tf == WESTON_TF_SRGB) { tmp = *in_prof->params; - tmp.tf_info = weston_color_tf_info_from(cm->base.compositor, WESTON_TF_GAMMA22); + tmp.tf.info = weston_color_tf_info_from(cm->base.compositor, WESTON_TF_GAMMA22); ret = cmlcms_get_color_profile_from_params(&cm->base, &tmp, "override sRGB EOTF", &replacement, &errmsg); @@ -1972,8 +1984,8 @@ cmclms_adjust_recipe(struct cmlcms_color_transform_recipe *adjusted, "Replacing profile p%u (%s) with profile p%u (%s)" "for color transformation.\n", in_prof->base.id, - in_prof->params->tf_info->desc, - replacement->id, tmp.tf_info->desc); + in_prof->params->tf.info->desc, + replacement->id, tmp.tf.info->desc); unref_cprof(adjusted->input_profile); adjusted->input_profile = to_cmlcms_cprof(replacement); } else { diff --git a/libweston/color-operations.c b/libweston/color-operations.c index 92ede7fd6..7628ba554 100644 --- a/libweston/color-operations.c +++ b/libweston/color-operations.c @@ -195,7 +195,7 @@ weston_color_curve_sample(struct weston_compositor *compositor, * Otherwise, fallback to a parametric curve and we'll handle * that below. */ - switch(curve->u.enumerated.tf->tf) { + switch (curve->u.enumerated.tf.info->tf) { case WESTON_TF_ST2084_PQ: sample_pq(curve->u.enumerated.tf_direction, ch, len, in, out); return true; diff --git a/libweston/color-profile-param-builder.c b/libweston/color-profile-param-builder.c index 5957a9b8f..c917028d2 100644 --- a/libweston/color-profile-param-builder.c +++ b/libweston/color-profile-param-builder.c @@ -310,9 +310,9 @@ weston_color_profile_param_builder_set_tf_named(struct weston_color_profile_para if (!success) return false; - builder->params.tf_info = weston_color_tf_info_from(compositor, tf); + builder->params.tf.info = weston_color_tf_info_from(compositor, tf); weston_assert_uint_eq(builder->compositor, - builder->params.tf_info->count_parameters, 0); + builder->params.tf.info->count_parameters, 0); builder->group_mask |= WESTON_COLOR_PROFILE_PARAMS_TF; @@ -368,8 +368,8 @@ weston_color_profile_param_builder_set_tf_power_exponent(struct weston_color_pro if (!success) return false; - builder->params.tf_info = weston_color_tf_info_from(compositor, WESTON_TF_POWER); - builder->params.tf_params[0] = power_exponent; + builder->params.tf.info = weston_color_tf_info_from(compositor, WESTON_TF_POWER); + builder->params.tf.params[0] = power_exponent; builder->group_mask |= WESTON_COLOR_PROFILE_PARAMS_TF; @@ -757,7 +757,7 @@ builder_complete_params(struct weston_color_profile_param_builder *builder) /* Some TF's override the default. Values comes from the CM&HDR * protocol as well. */ if (builder->group_mask & WESTON_COLOR_PROFILE_PARAMS_TF) { - switch(builder->params.tf_info->tf) { + switch(builder->params.tf.info->tf) { case WESTON_TF_BT1886: builder->params.reference_white_luminance = 100.0; builder->params.min_luminance = 0.01; @@ -781,7 +781,7 @@ builder_complete_params(struct weston_color_profile_param_builder *builder) /* Primary luminance is set, but the CM&HDR protocol states that * PQ TF should override max_lum with min_lum + 10000 cd/m². */ if ((builder->group_mask & WESTON_COLOR_PROFILE_PARAMS_TF) && - builder->params.tf_info->tf == WESTON_TF_ST2084_PQ) + builder->params.tf.info->tf == WESTON_TF_ST2084_PQ) builder->params.max_luminance = builder->params.min_luminance + 10000.0; } diff --git a/libweston/color.c b/libweston/color.c index 762fc7474..0ce961311 100644 --- a/libweston/color.c +++ b/libweston/color.c @@ -166,12 +166,12 @@ weston_color_profile_params_to_str(struct weston_color_profile_params *params, if (params->primaries_info) fprintf(fp, "%sprimaries named: %s\n", ident, params->primaries_info->desc); - fprintf(fp, "%stransfer function: %s\n", ident, params->tf_info->desc); + fprintf(fp, "%stransfer function: %s\n", ident, params->tf.info->desc); - if (params->tf_info->count_parameters > 0) { + if (params->tf.info->count_parameters > 0) { fprintf(fp, "%s params:", ident); - for (i = 0; i < params->tf_info->count_parameters; i++) - fprintf(fp, " %.4f", params->tf_params[i]); + for (i = 0; i < params->tf.info->count_parameters; i++) + fprintf(fp, " %.4f", params->tf.params[i]); fprintf(fp, "\n"); } @@ -214,11 +214,11 @@ weston_color_curve_enum_get_parametric(struct weston_compositor *compositor, memset(out, 0, sizeof(*out)); /* This one is special, the only parametric TF we currently have. */ - if (curve->tf->tf == WESTON_TF_POWER) { + if (curve->tf.info->tf == WESTON_TF_POWER) { out->type = WESTON_COLOR_CURVE_PARAMETRIC_TYPE_LINPOW; out->clamped_input = false; for (i = 0; i < 3; i++) { - float exp = curve->params[i][0]; + float exp = curve->tf.params[0]; /* LINPOW with such params matches pure power-law */ out->params.chan[i].g = (curve->tf_direction == WESTON_FORWARD_TF) ? exp : 1.0f / exp; @@ -231,15 +231,15 @@ weston_color_curve_enum_get_parametric(struct weston_compositor *compositor, } /* No other TF's have params. */ - weston_assert_uint_eq(compositor, curve->tf->count_parameters, 0); + weston_assert_uint_eq(compositor, curve->tf.info->count_parameters, 0); - if (!curve->tf->curve_params_valid) + if (!curve->tf.info->curve_params_valid) return false; if (curve->tf_direction == WESTON_FORWARD_TF) - *out = curve->tf->curve; + *out = curve->tf.info->curve; else - *out = curve->tf->inverse_curve; + *out = curve->tf.info->inverse_curve; return true; } @@ -254,9 +254,9 @@ curve_to_lut_has_good_precision(struct weston_color_curve *curve) if (curve->type == WESTON_COLOR_CURVE_TYPE_ENUM) { if (e->tf_direction == WESTON_INVERSE_TF) { - if (e->tf->tf == WESTON_TF_ST2084_PQ || - e->tf->tf == WESTON_TF_GAMMA22 || - e->tf->tf == WESTON_TF_GAMMA28) { + if (e->tf.info->tf == WESTON_TF_ST2084_PQ || + e->tf.info->tf == WESTON_TF_GAMMA22 || + e->tf.info->tf == WESTON_TF_GAMMA28) { /** * These have bad precision in the indirect * direction. @@ -264,30 +264,26 @@ curve_to_lut_has_good_precision(struct weston_color_curve *curve) return false; } - if (e->tf->tf == WESTON_TF_POWER) { + if (e->tf.info->tf == WESTON_TF_POWER) { /** * Same as the above, but for parametric * power-law transfer function. If g > 1.0 * it would result in bad precision. */ - for (i = 0; i < 3; i++) { - g = e->params[i][0]; - if (g > 1.0f) - return false; - } + g = e->tf.params[0]; + if (g > 1.0f) + return false; } } else { - if (e->tf->tf == WESTON_TF_POWER) { + if (e->tf.info->tf == WESTON_TF_POWER) { /** * For parametric power-law transfer function * in the forward direction, g < 1.0 would * result in bad precision. */ - for (i = 0; i < 3; i++) { - g = e->params[i][0]; - if (g < 1.0f) - return false; - } + g = e->tf.params[0]; + if (g < 1.0f) + return false; } } } else if (curve->type == WESTON_COLOR_CURVE_TYPE_PARAMETRIC) { @@ -573,16 +569,14 @@ weston_color_curve_details_fprint(FILE *fp, break; case WESTON_COLOR_CURVE_TYPE_ENUM: en = &curve->u.enumerated; - if (en->tf->count_parameters == 0) + if (en->tf.info->count_parameters == 0) break; - fprintf(fp, "%*s%s, %s:\n", indent, "", step, en->tf->desc); - for (ch = 0; ch < 3; ch++) { - fprintf(fp, "%*s %s", indent, "", chan[ch]); - for (i = 0; i < en->tf->count_parameters; i++) - fprintf(fp, " % .4f", en->params[ch][i]); - fprintf(fp, "\n"); - } + fprintf(fp, "%*s%s, %s:\n", indent, "", step, en->tf.info->desc); + fprintf(fp, "%*s R,G,B", indent, ""); + for (i = 0; i < en->tf.info->count_parameters; i++) + fprintf(fp, " % .4f", en->tf.params[i]); + fprintf(fp, "\n"); break; case WESTON_COLOR_CURVE_TYPE_PARAMETRIC: par = &curve->u.parametric; @@ -674,7 +668,7 @@ weston_color_curve_fprint(FILE *fp, const struct weston_color_curve *curve) case WESTON_COLOR_CURVE_TYPE_ENUM: fprintf(fp, "(enum) %s%s", curve->u.enumerated.tf_direction == WESTON_INVERSE_TF ? "inverse " : "", - curve->u.enumerated.tf->desc); + curve->u.enumerated.tf.info->desc); break; case WESTON_COLOR_CURVE_TYPE_PARAMETRIC: fprintf(fp, "(parametric) %s", diff --git a/libweston/color.h b/libweston/color.h index 59af55514..0e98538e9 100644 --- a/libweston/color.h +++ b/libweston/color.h @@ -33,11 +33,6 @@ #include "backend-drm/drm-kms-enums.h" -/** - * The only tf which is parametric (WESTON_TF_POWER) has a single parameter. - */ -#define MAX_PARAMS_TF 1 - enum weston_hdr_metadata_type1_groups { /** weston_hdr_metadata_type1::primary is set */ WESTON_HDR_METADATA_TYPE1_GROUP_PRIMARIES = 0x01, @@ -133,6 +128,21 @@ struct weston_color_profile { uint32_t id; }; +/** + * Color transfer function + * + * Includes any parameter values the enumerated transfer function might need. + */ +struct weston_color_tf { + /** Encoding transfer characteristic by enumeration; always set. */ + const struct weston_color_tf_info *info; + + /** TF parameters, specific to TF. */ + float params[1]; + + char padding[4]; +}; + /** Parameters that define a parametric color profile */ struct weston_color_profile_params { /* Primary color volume; always set. */ @@ -142,10 +152,7 @@ struct weston_color_profile_params { const struct weston_color_primaries_info *primaries_info; /* Encoding transfer characteristic by enumeration; always set. */ - const struct weston_color_tf_info *tf_info; - - /* Transfer characteristic's parameters; depends on tf_info. */ - float tf_params[MAX_PARAMS_TF]; + struct weston_color_tf tf; /* Primary color volume luminance parameters cd/m²; always set. */ float min_luminance, max_luminance; @@ -158,7 +165,7 @@ struct weston_color_profile_params { float target_min_luminance, target_max_luminance; float maxCLL, maxFALL; - char padding[8]; + char padding[4]; }; /** Type for parametric curves */ @@ -257,14 +264,10 @@ enum weston_tf_direction { /** Enumerated color curve */ struct weston_color_curve_enum { - const struct weston_color_tf_info *tf; + struct weston_color_tf tf; /* Determines if the direct or inverse of the tf should be used. */ enum weston_tf_direction tf_direction; - - /* Some tf are parametric, and we keep the params here. They may be - * different for each color channel, and channels are in RGB order. */ - float params[3][MAX_PARAMS_TF]; }; /** diff --git a/libweston/renderer-gl/gl-shader-config-color-transformation.c b/libweston/renderer-gl/gl-shader-config-color-transformation.c index 2dd7a13f9..26fbac6e5 100644 --- a/libweston/renderer-gl/gl-shader-config-color-transformation.c +++ b/libweston/renderer-gl/gl-shader-config-color-transformation.c @@ -172,7 +172,7 @@ gl_color_curve_enum(struct gl_renderer *gr, * Handle enum curve (if TF is implemented) or fallback to a parametric * curve. */ - switch(curve->u.enumerated.tf->tf) { + switch (curve->u.enumerated.tf.info->tf) { case WESTON_TF_ST2084_PQ: gl_curve->type = (curve->u.enumerated.tf_direction == WESTON_FORWARD_TF) ? SHADER_COLOR_CURVE_PQ : SHADER_COLOR_CURVE_PQ_INVERSE;