From 04cb3c1bbd5b3ff96ea814be179778ca2fe4c8b5 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Mon, 28 Jul 2025 18:39:01 +0200 Subject: [PATCH] VSE: Faster Histogram scope calculation Histogram was calculated by copying the rendered image, transforming it into display space, and calculating the histogram on that. On large resolutions, this copy+transform+free of the temporary image was taking up majority of the time. Especially for default use case when the display transform is a no-op. Change the code so that display transform, if needed, is done directly inside histogram calculation, without needing a full-size temporary image. Performance of histogram calculation, on Ryzen 5950X (Windows), on a 4K resolution image, with default color management settings: - PNG (SDR): 3.9 -> 0.9ms - EXR (HDR): 41.3 -> 6.3ms With display colorspace (P3) that is different than the sequencer colorspace (sRGB): - PNG (SDR): 25.3 -> 11.3ms - EXR (HDR): 64.9 -> 10.6ms It also fixes calculation of histogram on float (HDR) images that have alpha channel; the histogram was wrongly calculated on premultiplied color values, which was not consistent with how it was calculated on the byte images. Pull Request: https://projects.blender.org/blender/blender/pulls/143175 --- .../space_sequencer/sequencer_preview_draw.cc | 6 +- .../space_sequencer/sequencer_scopes.cc | 73 +++++++++++++++---- .../space_sequencer/sequencer_scopes.hh | 6 +- source/blender/imbuf/IMB_colormanagement.hh | 20 ++--- .../blender/imbuf/intern/colormanagement.cc | 67 +++++------------ 5 files changed, 89 insertions(+), 83 deletions(-) diff --git a/source/blender/editors/space_sequencer/sequencer_preview_draw.cc b/source/blender/editors/space_sequencer/sequencer_preview_draw.cc index c969e23b29f..e3279da9bad 100644 --- a/source/blender/editors/space_sequencer/sequencer_preview_draw.cc +++ b/source/blender/editors/space_sequencer/sequencer_preview_draw.cc @@ -839,11 +839,7 @@ static bool sequencer_calc_scopes(const SpaceSeq &space_sequencer, } break; case SEQ_DRAW_IMG_HISTOGRAM: { - ImBuf *display_ibuf = IMB_dupImBuf(&ibuf); - IMB_colormanagement_imbuf_make_display_space( - display_ibuf, &view_settings, &display_settings); - scopes->histogram.calc_from_ibuf(display_ibuf); - IMB_freeImBuf(display_ibuf); + scopes->histogram.calc_from_ibuf(&ibuf, view_settings, display_settings); } break; case SEQ_DRAW_IMG_RGBPARADE: if (!scopes->sep_waveform_ibuf) { diff --git a/source/blender/editors/space_sequencer/sequencer_scopes.cc b/source/blender/editors/space_sequencer/sequencer_scopes.cc index 8ac244672f0..85833d54774 100644 --- a/source/blender/editors/space_sequencer/sequencer_scopes.cc +++ b/source/blender/editors/space_sequencer/sequencer_scopes.cc @@ -281,45 +281,88 @@ static int get_bin_float(float f) return clamp_i(bin, 0, ScopeHistogram::BINS_FLOAT - 1); } -void ScopeHistogram::calc_from_ibuf(const ImBuf *ibuf) +void ScopeHistogram::calc_from_ibuf(const ImBuf *ibuf, + const ColorManagedViewSettings &view_settings, + const ColorManagedDisplaySettings &display_settings) { #ifdef DEBUG_TIME SCOPED_TIMER(__func__); #endif + ColormanageProcessor *cm_processor = IMB_colormanagement_display_processor_for_imbuf( + ibuf, &view_settings, &display_settings); + const bool is_float = ibuf->float_buffer.data != nullptr; const int hist_size = is_float ? BINS_FLOAT : BINS_BYTE; Array counts(hist_size, uint3(0)); data = threading::parallel_reduce( - IndexRange(ibuf->y), - 256, + IndexRange(IMB_get_pixel_count(ibuf)), + 16 * 1024, counts, - [&](const IndexRange y_range, const Array &init) { + [&](const IndexRange range, const Array &init) { Array res = init; if (is_float) { - for (const int y : y_range) { - const float *src = ibuf->float_buffer.data + y * ibuf->x * 4; - for (int x = 0; x < ibuf->x; x++) { - res[get_bin_float(src[0])].x++; - res[get_bin_float(src[1])].y++; - res[get_bin_float(src[2])].z++; + const float *src = ibuf->float_buffer.data + range.first() * 4; + if (!cm_processor) { + /* Float image, no color space conversions needed. */ + for ([[maybe_unused]] const int64_t index : range) { + float4 pixel; + premul_to_straight_v4_v4(pixel, src); + res[get_bin_float(pixel.x)].x++; + res[get_bin_float(pixel.y)].y++; + res[get_bin_float(pixel.z)].z++; src += 4; } } + else { + /* Float image, with color space conversions. */ + Array pixels(range.size(), NoInitialization()); + for (int64_t i : pixels.index_range()) { + premul_to_straight_v4_v4(pixels[i], src + i * 4); + } + IMB_colormanagement_colorspace_to_scene_linear( + &pixels.data()->x, pixels.size(), 1, 4, ibuf->float_buffer.colorspace, false); + IMB_colormanagement_processor_apply( + cm_processor, &pixels.data()->x, pixels.size(), 1, 4, false); + for (const float4 &pixel : pixels) { + res[get_bin_float(pixel.x)].x++; + res[get_bin_float(pixel.y)].y++; + res[get_bin_float(pixel.z)].z++; + } + } } else { /* Byte images just use 256 histogram bins, directly indexed by value. */ - for (const int y : y_range) { - const uchar *src = ibuf->byte_buffer.data + y * ibuf->x * 4; - for (int x = 0; x < ibuf->x; x++) { + const uchar *src = ibuf->byte_buffer.data + range.first() * 4; + if (!cm_processor) { + /* Byte image, no color space conversions needed. */ + for ([[maybe_unused]] const int64_t index : range) { res[src[0]].x++; res[src[1]].y++; res[src[2]].z++; src += 4; } } + else { + /* Byte image, with color space conversions. */ + Array pixels(range.size(), NoInitialization()); + for (int64_t i : pixels.index_range()) { + rgba_uchar_to_float(pixels[i], src + i * 4); + } + IMB_colormanagement_colorspace_to_scene_linear( + &pixels.data()->x, pixels.size(), 1, 4, ibuf->byte_buffer.colorspace, false); + IMB_colormanagement_processor_apply( + cm_processor, &pixels.data()->x, pixels.size(), 1, 4, false); + for (const float4 &pixel : pixels) { + uchar pixel_b[4]; + rgba_float_to_uchar(pixel_b, pixel); + res[pixel_b[0]].x++; + res[pixel_b[1]].y++; + res[pixel_b[2]].z++; + } + } } return res; }, @@ -332,6 +375,10 @@ void ScopeHistogram::calc_from_ibuf(const ImBuf *ibuf) return res; }); + if (cm_processor) { + IMB_colormanagement_processor_free(cm_processor); + } + max_value = uint3(0); for (const uint3 &v : data) { max_value = math::max(max_value, v); diff --git a/source/blender/editors/space_sequencer/sequencer_scopes.hh b/source/blender/editors/space_sequencer/sequencer_scopes.hh index c3466a78012..38a819a4ec6 100644 --- a/source/blender/editors/space_sequencer/sequencer_scopes.hh +++ b/source/blender/editors/space_sequencer/sequencer_scopes.hh @@ -12,6 +12,8 @@ #include "BLI_math_vector_types.hh" #include "BLI_utility_mixins.hh" +struct ColorManagedViewSettings; +struct ColorManagedDisplaySettings; struct ImBuf; namespace blender::ed::vse { @@ -26,7 +28,9 @@ struct ScopeHistogram { Array data; uint3 max_value; - void calc_from_ibuf(const ImBuf *ibuf); + void calc_from_ibuf(const ImBuf *ibuf, + const ColorManagedViewSettings &view_settings, + const ColorManagedDisplaySettings &display_settings); bool is_float_hist() const { return data.size() == BINS_FLOAT; diff --git a/source/blender/imbuf/IMB_colormanagement.hh b/source/blender/imbuf/IMB_colormanagement.hh index 21fe2c3f2e7..37814ef3a70 100644 --- a/source/blender/imbuf/IMB_colormanagement.hh +++ b/source/blender/imbuf/IMB_colormanagement.hh @@ -253,12 +253,6 @@ void IMB_colormanagement_pixel_to_display_space_v4( const ColorManagedViewSettings *view_settings, const ColorManagedDisplaySettings *display_settings); -void IMB_colormanagement_pixel_to_display_space_v3( - float result[3], - const float pixel[3], - const ColorManagedViewSettings *view_settings, - const ColorManagedDisplaySettings *display_settings); - void IMB_colormanagement_imbuf_make_display_space( ImBuf *ibuf, const ColorManagedViewSettings *view_settings, @@ -312,14 +306,6 @@ void IMB_display_buffer_transform_apply(unsigned char *display_buffer, const ColorManagedViewSettings *view_settings, const ColorManagedDisplaySettings *display_settings, bool predivide); -void IMB_display_buffer_transform_apply_float(float *float_display_buffer, - float *linear_buffer, - int width, - int height, - int channels, - const ColorManagedViewSettings *view_settings, - const ColorManagedDisplaySettings *display_settings, - bool predivide); void IMB_display_buffer_release(void *cache_handle); @@ -436,6 +422,12 @@ void IMB_partial_display_buffer_update_delayed( ColormanageProcessor *IMB_colormanagement_display_processor_new( const ColorManagedViewSettings *view_settings, const ColorManagedDisplaySettings *display_settings); + +ColormanageProcessor *IMB_colormanagement_display_processor_for_imbuf( + const ImBuf *ibuf, + const ColorManagedViewSettings *view_settings, + const ColorManagedDisplaySettings *display_settings); + ColormanageProcessor *IMB_colormanagement_colorspace_processor_new(const char *from_colorspace, const char *to_colorspace); bool IMB_colormanagement_processor_is_noop(ColormanageProcessor *cm_processor); diff --git a/source/blender/imbuf/intern/colormanagement.cc b/source/blender/imbuf/intern/colormanagement.cc index 1981be6e8bd..862502657fb 100644 --- a/source/blender/imbuf/intern/colormanagement.cc +++ b/source/blender/imbuf/intern/colormanagement.cc @@ -1475,7 +1475,7 @@ static void do_display_buffer_apply_no_processor(DisplayBufferThread *handle) width, width); } - else if (handle->buffer) { + else if (handle->buffer && handle->display_buffer != handle->buffer) { IMB_buffer_float_from_float(handle->display_buffer, handle->buffer, handle->channels, @@ -1620,15 +1620,11 @@ static bool is_colorspace_same_as_display(const ColorSpace *colorspace, return false; } -static void colormanage_display_buffer_process_ex( - ImBuf *ibuf, - float *display_buffer, - uchar *display_buffer_byte, +ColormanageProcessor *IMB_colormanagement_display_processor_for_imbuf( + const ImBuf *ibuf, const ColorManagedViewSettings *view_settings, const ColorManagedDisplaySettings *display_settings) { - ColormanageProcessor *cm_processor = nullptr; - /* Check if we can skip colorspace transforms. */ bool skip_transform = false; if (ibuf->float_buffer.data == nullptr && ibuf->byte_buffer.colorspace) { skip_transform = is_colorspace_same_as_display( @@ -1638,11 +1634,22 @@ static void colormanage_display_buffer_process_ex( skip_transform = is_colorspace_same_as_display( ibuf->float_buffer.colorspace, view_settings, display_settings); } - - if (skip_transform == false) { - cm_processor = IMB_colormanagement_display_processor_new(view_settings, display_settings); + if (skip_transform) { + return nullptr; } + return IMB_colormanagement_display_processor_new(view_settings, display_settings); +} + +static void colormanage_display_buffer_process_ex( + ImBuf *ibuf, + float *display_buffer, + uchar *display_buffer_byte, + const ColorManagedViewSettings *view_settings, + const ColorManagedDisplaySettings *display_settings) +{ + ColormanageProcessor *cm_processor = IMB_colormanagement_display_processor_for_imbuf( + ibuf, view_settings, display_settings); display_buffer_apply_threaded(ibuf, ibuf->float_buffer.data, ibuf->byte_buffer.data, @@ -2250,21 +2257,6 @@ void IMB_colormanagement_pixel_to_display_space_v4( IMB_colormanagement_processor_free(cm_processor); } -void IMB_colormanagement_pixel_to_display_space_v3( - float result[3], - const float pixel[3], - const ColorManagedViewSettings *view_settings, - const ColorManagedDisplaySettings *display_settings) -{ - ColormanageProcessor *cm_processor; - - copy_v3_v3(result, pixel); - - cm_processor = IMB_colormanagement_display_processor_new(view_settings, display_settings); - IMB_colormanagement_processor_apply_v3(cm_processor, result); - IMB_colormanagement_processor_free(cm_processor); -} - static void colormanagement_imbuf_make_display_space( ImBuf *ibuf, const ColorManagedViewSettings *view_settings, @@ -2605,31 +2597,6 @@ void IMB_display_buffer_transform_apply(uchar *display_buffer, MEM_freeN(buffer); } -void IMB_display_buffer_transform_apply_float(float *float_display_buffer, - float *linear_buffer, - int width, - int height, - int channels, - const ColorManagedViewSettings *view_settings, - const ColorManagedDisplaySettings *display_settings, - bool predivide) -{ - float *buffer; - ColormanageProcessor *cm_processor = IMB_colormanagement_display_processor_new(view_settings, - display_settings); - - buffer = MEM_malloc_arrayN(size_t(channels) * size_t(width) * size_t(height), - "display transform temp buffer"); - memcpy(buffer, linear_buffer, size_t(channels) * width * height * sizeof(float)); - - IMB_colormanagement_processor_apply(cm_processor, buffer, width, height, channels, predivide); - - IMB_colormanagement_processor_free(cm_processor); - - memcpy(float_display_buffer, buffer, size_t(channels) * width * height * sizeof(float)); - MEM_freeN(buffer); -} - void IMB_display_buffer_release(void *cache_handle) { if (cache_handle) {