From ebe67e5ca04c4e7b4d104cb2c481747163626849 Mon Sep 17 00:00:00 2001 From: Omar Emara Date: Mon, 5 May 2025 10:51:34 +0200 Subject: [PATCH] Compositor: Remove Premultiplied option from Alpha Over node This patch removes the Premultiplied option from the Alpha Over node. The reasoning is as follows: - The option mixed between alpha being straight and premultiplied, a state which doesn't happen in practice. - The option conflicts with the Convert Premultiplied option, if it is not zero, then Convert Premultiplied does nothing. - The option is implemented in a lossy way. It premultiplies the alpha assuming it is straight, then converts the result to straight again, then mixed between both results. This is as opposed to mixing the original straight input with the premultiplied input. The successive alpha conversion causes data loss in transparent regions. Pull Request: https://projects.blender.org/blender/blender/pulls/138428 --- .../blender/blenkernel/BKE_blender_version.h | 2 +- .../blenloader/intern/versioning_400.cc | 89 +++++++++++++++++++ .../gpu_shader_compositor_alpha_over.glsl | 18 ---- .../blender/makesrna/intern/rna_nodetree.cc | 2 +- .../nodes/node_composite_alpha_over.cc | 57 +----------- 5 files changed, 94 insertions(+), 74 deletions(-) diff --git a/source/blender/blenkernel/BKE_blender_version.h b/source/blender/blenkernel/BKE_blender_version.h index a9b206268aa..f2856c87cff 100644 --- a/source/blender/blenkernel/BKE_blender_version.h +++ b/source/blender/blenkernel/BKE_blender_version.h @@ -27,7 +27,7 @@ /* Blender file format version. */ #define BLENDER_FILE_VERSION BLENDER_VERSION -#define BLENDER_FILE_SUBVERSION 62 +#define BLENDER_FILE_SUBVERSION 63 /* Minimum Blender version that supports reading file written with the current * version. Older Blender versions will test this and cancel loading the file, showing a warning to diff --git a/source/blender/blenloader/intern/versioning_400.cc b/source/blender/blenloader/intern/versioning_400.cc index 7d6f4757f5c..638aabba9a6 100644 --- a/source/blender/blenloader/intern/versioning_400.cc +++ b/source/blender/blenloader/intern/versioning_400.cc @@ -4428,6 +4428,86 @@ static void do_version_bright_contrast_remove_premultiplied(bNodeTree *node_tree } } +/* The Premultiply Mix option was removed. If enabled, the image is converted to premultiplied then + * to straight, and both are mixed using a mix node. */ +static void do_version_alpha_over_remove_premultiply(bNodeTree *node_tree) +{ + LISTBASE_FOREACH_BACKWARD_MUTABLE (bNodeLink *, link, &node_tree->links) { + if (link->tonode->type_legacy != CMP_NODE_ALPHAOVER) { + continue; + } + + const float mix_factor = static_cast(link->tonode->storage)->x; + if (mix_factor == 0.0f) { + continue; + } + + if (blender::StringRef(link->tosock->identifier) != "Image_001") { + continue; + } + + /* Disable Convert Premultiplied option, since this will be done manually. */ + link->tonode->custom1 = false; + + bNode *mix_node = blender::bke::node_add_static_node(nullptr, *node_tree, SH_NODE_MIX); + mix_node->parent = link->tonode->parent; + mix_node->location[0] = link->tonode->location[0] - link->tonode->width - 20.0f; + mix_node->location[1] = link->tonode->location[1]; + static_cast(mix_node->storage)->data_type = SOCK_RGBA; + + bNodeSocket *mix_a_input = blender::bke::node_find_socket(*mix_node, SOCK_IN, "A_Color"); + bNodeSocket *mix_b_input = blender::bke::node_find_socket(*mix_node, SOCK_IN, "B_Color"); + bNodeSocket *mix_factor_input = blender::bke::node_find_socket( + *mix_node, SOCK_IN, "Factor_Float"); + bNodeSocket *mix_output = blender::bke::node_find_socket(*mix_node, SOCK_OUT, "Result_Color"); + + mix_factor_input->default_value_typed()->value = mix_factor; + + bNode *to_straight_node = blender::bke::node_add_static_node( + nullptr, *node_tree, CMP_NODE_PREMULKEY); + to_straight_node->parent = link->tonode->parent; + to_straight_node->location[0] = mix_node->location[0] - mix_node->width - 20.0f; + to_straight_node->location[1] = mix_node->location[1]; + to_straight_node->custom1 = CMP_NODE_ALPHA_CONVERT_UNPREMULTIPLY; + + bNodeSocket *to_straight_input = blender::bke::node_find_socket( + *to_straight_node, SOCK_IN, "Image"); + bNodeSocket *to_straight_output = blender::bke::node_find_socket( + *to_straight_node, SOCK_OUT, "Image"); + + bNode *to_premultiplied_node = blender::bke::node_add_static_node( + nullptr, *node_tree, CMP_NODE_PREMULKEY); + to_premultiplied_node->parent = link->tonode->parent; + to_premultiplied_node->location[0] = to_straight_node->location[0] - to_straight_node->width - + 20.0f; + to_premultiplied_node->location[1] = to_straight_node->location[1]; + to_premultiplied_node->custom1 = CMP_NODE_ALPHA_CONVERT_PREMULTIPLY; + + bNodeSocket *to_premultiplied_input = blender::bke::node_find_socket( + *to_premultiplied_node, SOCK_IN, "Image"); + bNodeSocket *to_premultiplied_output = blender::bke::node_find_socket( + *to_premultiplied_node, SOCK_OUT, "Image"); + + version_node_add_link(*node_tree, + *link->fromnode, + *link->fromsock, + *to_premultiplied_node, + *to_premultiplied_input); + version_node_add_link(*node_tree, + *to_premultiplied_node, + *to_premultiplied_output, + *to_straight_node, + *to_straight_input); + version_node_add_link( + *node_tree, *to_premultiplied_node, *to_premultiplied_output, *mix_node, *mix_b_input); + version_node_add_link( + *node_tree, *to_straight_node, *to_straight_output, *mix_node, *mix_a_input); + version_node_add_link(*node_tree, *mix_node, *mix_output, *link->tonode, *link->tosock); + + blender::bke::node_remove_link(node_tree, *link); + } +} + static void do_version_viewer_shortcut(bNodeTree *node_tree) { LISTBASE_FOREACH_MUTABLE (bNode *, node, &node_tree->nodes) { @@ -10461,6 +10541,15 @@ void blo_do_versions_400(FileData *fd, Library * /*lib*/, Main *bmain) FOREACH_NODETREE_END; } + if (!MAIN_VERSION_FILE_ATLEAST(bmain, 405, 63)) { + FOREACH_NODETREE_BEGIN (bmain, node_tree, id) { + if (node_tree->type == NTREE_COMPOSIT) { + do_version_alpha_over_remove_premultiply(node_tree); + } + } + FOREACH_NODETREE_END; + } + /* Always run this versioning (keep at the bottom of the function). Meshes are written with the * legacy format which always needs to be converted to the new format on file load. To be moved * to a subversion check in 5.0. */ diff --git a/source/blender/compositor/shaders/library/gpu_shader_compositor_alpha_over.glsl b/source/blender/compositor/shaders/library/gpu_shader_compositor_alpha_over.glsl index fcd5a558253..4d034971953 100644 --- a/source/blender/compositor/shaders/library/gpu_shader_compositor_alpha_over.glsl +++ b/source/blender/compositor/shaders/library/gpu_shader_compositor_alpha_over.glsl @@ -2,24 +2,6 @@ * * SPDX-License-Identifier: GPL-2.0-or-later */ -void node_composite_alpha_over_mixed( - float factor, float4 color, float4 over_color, float premultiply_factor, out float4 result) -{ - if (over_color.a <= 0.0f) { - result = color; - } - else if (factor == 1.0f && over_color.a >= 1.0f) { - result = over_color; - } - else { - float add_factor = 1.0f - premultiply_factor + over_color.a * premultiply_factor; - float premultiplier = factor * add_factor; - float multiplier = 1.0f - factor * over_color.a; - - result = multiplier * color + float2(premultiplier, factor).xxxy * over_color; - } -} - void node_composite_alpha_over_key(float factor, float4 color, float4 over_color, diff --git a/source/blender/makesrna/intern/rna_nodetree.cc b/source/blender/makesrna/intern/rna_nodetree.cc index 0153c8deb75..22917a0946d 100644 --- a/source/blender/makesrna/intern/rna_nodetree.cc +++ b/source/blender/makesrna/intern/rna_nodetree.cc @@ -7296,7 +7296,7 @@ static void def_cmp_alpha_over(BlenderRNA * /*brna*/, StructRNA *srna) prop = RNA_def_property(srna, "premul", PROP_FLOAT, PROP_FACTOR); RNA_def_property_float_sdna(prop, nullptr, "x"); RNA_def_property_range(prop, 0.0f, 1.0f); - RNA_def_property_ui_text(prop, "Premultiplied", "Mix Factor"); + RNA_def_property_ui_text(prop, "Premultiplied", "Mix Factor. (Deprecated: Unused.)"); RNA_def_property_update(prop, NC_NODE | NA_EDITED, "rna_Node_update"); } diff --git a/source/blender/nodes/composite/nodes/node_composite_alpha_over.cc b/source/blender/nodes/composite/nodes/node_composite_alpha_over.cc index 0f15aea2e3c..2fceed0eed8 100644 --- a/source/blender/nodes/composite/nodes/node_composite_alpha_over.cc +++ b/source/blender/nodes/composite/nodes/node_composite_alpha_over.cc @@ -24,8 +24,6 @@ namespace blender::nodes::node_composite_alpha_over_cc { -NODE_STORAGE_FUNCS(NodeTwoFloats) - static void cmp_node_alphaover_declare(NodeDeclarationBuilder &b) { b.add_input("Fac") @@ -45,16 +43,13 @@ static void cmp_node_alphaover_declare(NodeDeclarationBuilder &b) static void node_alphaover_init(bNodeTree * /*ntree*/, bNode *node) { + /* Not used, but the data is still allocated for forward compatibility. */ node->storage = MEM_callocN(__func__); } static void node_composit_buts_alphaover(uiLayout *layout, bContext * /*C*/, PointerRNA *ptr) { - uiLayout *col; - - col = &layout->column(true); - uiItemR(col, ptr, "use_premultiply", UI_ITEM_R_SPLIT_EMPTY_NAME, std::nullopt, ICON_NONE); - uiItemR(col, ptr, "premul", UI_ITEM_R_SPLIT_EMPTY_NAME, std::nullopt, ICON_NONE); + uiItemR(layout, ptr, "use_premultiply", UI_ITEM_R_SPLIT_EMPTY_NAME, std::nullopt, ICON_NONE); } using namespace blender::compositor; @@ -64,27 +59,12 @@ static bool get_use_premultiply(const bNode &node) return node.custom1; } -static float get_premultiply_factor(const bNode &node) -{ - return node_storage(node).x; -} - static int node_gpu_material(GPUMaterial *material, bNode *node, bNodeExecData * /*execdata*/, GPUNodeStack *inputs, GPUNodeStack *outputs) { - const float premultiply_factor = get_premultiply_factor(*node); - if (premultiply_factor != 0.0f) { - return GPU_stack_link(material, - node, - "node_composite_alpha_over_mixed", - inputs, - outputs, - GPU_uniform(&premultiply_factor)); - } - if (get_use_premultiply(*node)) { return GPU_stack_link(material, node, "node_composite_alpha_over_key", inputs, outputs); } @@ -92,26 +72,6 @@ static int node_gpu_material(GPUMaterial *material, return GPU_stack_link(material, node, "node_composite_alpha_over_premultiply", inputs, outputs); } -static float4 alpha_over_mixed(const float factor, - const float4 &color, - const float4 &over_color, - const float premultiply_factor) -{ - if (over_color.w <= 0.0f) { - return color; - } - - if (factor == 1.0f && over_color.w >= 1.0f) { - return over_color; - } - - float add_factor = 1.0f - premultiply_factor + over_color.w * premultiply_factor; - float premultiplier = factor * add_factor; - float multiplier = 1.0f - factor * over_color.w; - - return multiplier * color + float4(float3(premultiplier), factor) * over_color; -} - static float4 alpha_over_key(const float factor, const float4 &color, const float4 &over_color) { if (over_color.w <= 0.0f) { @@ -157,18 +117,7 @@ static void node_build_multi_function(blender::nodes::NodeMultiFunctionBuilder & }, mf::build::exec_presets::SomeSpanOrSingle<1, 2>()); - const float premultiply_factor = get_premultiply_factor(builder.node()); - if (premultiply_factor != 0.0f) { - builder.construct_and_set_matching_fn_cb([=]() { - return mf::build::SI3_SO( - "Alpha Over Mixed", - [=](const float factor, const float4 &color, const float4 &over_color) -> float4 { - return alpha_over_mixed(factor, color, over_color, premultiply_factor); - }, - mf::build::exec_presets::SomeSpanOrSingle<1, 2>()); - }); - } - else if (get_use_premultiply(builder.node())) { + if (get_use_premultiply(builder.node())) { builder.set_matching_fn(key_function); } else {