Compositor: Unify Bilateral Blur across CPU and GPU

This patch unifies the implementation of the Bilateral Blur node across
CPU and GPU. The difference is due to two things. First, the CPU code
had a bug where the upper limit of the blur window was not included in
the accumulation. Second, CPU ignored pixels outside of the image while
GPU clamped them to the nearest boundary pixel. The latter difference
was aligned with GPU until we eventually add an option to control
boundary handing.

A few utilities were added to the node operation and memory buffer
classes to do clamped pixel reading.

Pull Request: https://projects.blender.org/blender/blender/pulls/117751

source/blender/compositor/intern/COM_MemoryBuffer.h

@@ -194,11 +194,26 @@ class MemoryBuffer {
     return buffer_ + get_coords_offset(x, y);
   }
 
+  /**
+   * Get buffer element at given coordinates, clamped to border.
+   */
+  const float *get_elem_clamped(int x, int y) const
+  {
+    const int clamped_x = math::clamp(x, 0, this->get_width() - 1);
+    const int clamped_y = math::clamp(y, 0, this->get_height() - 1);
+    return buffer_ + get_coords_offset(clamped_x, clamped_y);
+  }
+
   void read_elem(int x, int y, float *out) const
   {
     memcpy(out, get_elem(x, y), get_elem_bytes_len());
   }
 
+  void read_elem_clamped(int x, int y, float *out) const
+  {
+    memcpy(out, get_elem_clamped(x, y), get_elem_bytes_len());
+  }
+
   void read_elem_checked(int x, int y, float *out) const
   {
     if (!has_coords(x, y)) {

source/blender/compositor/intern/COM_NodeOperation.h

@@ -9,6 +9,7 @@
 
 #include "BLI_ghash.h"
 #include "BLI_hash.hh"
+#include "BLI_math_base.hh"
 #include "BLI_rect.h"
 #include "BLI_span.hh"
 #include "BLI_threads.h"
@@ -591,6 +592,14 @@ class NodeOperation {
     execute_pixel(result, x, y, chunk_data);
   }
 
+  inline void read_clamped(float result[4], int x, int y, void *chunk_data)
+  {
+    execute_pixel(result,
+                  math::clamp(x, 0, int(this->get_width()) - 1),
+                  math::clamp(y, 0, int(this->get_height()) - 1),
+                  chunk_data);
+  }
+
   virtual void *initialize_tile_data(rcti * /*rect*/)
   {
     return 0;

source/blender/compositor/operations/COM_BilateralBlurOperation.cc

@@ -34,12 +34,7 @@ void BilateralBlurOperation::execute_pixel(float output[4], int x, int y, void *
   float temp_color[4];
   float blur_color[4];
   float blur_divider;
-  float space = space_;
   float sigmacolor = data_->sigma_color;
-  int minx = floor(x - space);
-  int maxx = ceil(x + space);
-  int miny = floor(y - space);
-  int maxy = ceil(y + space);
   float delta_color;
   input_determinator_program_->read(determinator_reference_color, x, y, data);
 
@@ -49,17 +44,17 @@ void BilateralBlurOperation::execute_pixel(float output[4], int x, int y, void *
    * using gaussian bell for weights. Also sigma_color doesn't seem to be
    * used correct at all.
    */
-  for (int yi = miny; yi < maxy; yi += QualityStepHelper::get_step()) {
-    for (int xi = minx; xi < maxx; xi += QualityStepHelper::get_step()) {
+  for (int yi = -radius_; yi <= radius_; yi += QualityStepHelper::get_step()) {
+    for (int xi = -radius_; xi <= radius_; xi += QualityStepHelper::get_step()) {
       /* Read determinator. */
-      input_determinator_program_->read(determinator, xi, yi, data);
+      input_determinator_program_->read_clamped(determinator, x + xi, y + yi, data);
       delta_color = (fabsf(determinator_reference_color[0] - determinator[0]) +
                      fabsf(determinator_reference_color[1] - determinator[1]) +
                      /* Do not take the alpha channel into account. */
                      fabsf(determinator_reference_color[2] - determinator[2]));
       if (delta_color < sigmacolor) {
         /* Add this to the blur. */
-        input_color_program_->read(temp_color, xi, yi, data);
+        input_color_program_->read_clamped(temp_color, x + xi, y + yi, data);
         add_v4_v4(blur_color, temp_color);
         blur_divider += 1.0f;
       }
@@ -87,7 +82,7 @@ bool BilateralBlurOperation::determine_depending_area_of_interest(
     rcti *input, ReadBufferOperation *read_operation, rcti *output)
 {
   rcti new_input;
-  int add = ceil(space_) + 1;
+  int add = radius_ + 1;
 
   new_input.xmax = input->xmax + (add);
   new_input.xmin = input->xmin - (add);
@@ -101,7 +96,7 @@ void BilateralBlurOperation::get_area_of_interest(const int /*input_idx*/,
                                                   const rcti &output_area,
                                                   rcti &r_input_area)
 {
-  const int add = ceil(space_) + 1;
+  const int add = radius_ + 1;
 
   r_input_area.xmax = output_area.xmax + (add);
   r_input_area.xmin = output_area.xmin - (add);
@@ -117,8 +112,9 @@ struct PixelCursor {
   const float *determ_reference_color;
   float temp_color[4];
   float *out;
-  int min_x, max_x;
-  int min_y, max_y;
+  int radius;
+  int x;
+  int y;
 };
 
 static void blur_pixel(PixelCursor &p)
@@ -130,16 +126,16 @@ static void blur_pixel(PixelCursor &p)
    * using gaussian bell for weights. Also sigma_color doesn't seem to be
    * used correct at all.
    */
-  for (int yi = p.min_y; yi < p.max_y; yi += p.step) {
-    for (int xi = p.min_x; xi < p.max_x; xi += p.step) {
-      p.input_determinator->read(p.temp_color, xi, yi);
+  for (int yi = -p.radius; yi <= p.radius; yi += p.step) {
+    for (int xi = -p.radius; xi <= p.radius; xi += p.step) {
+      p.input_determinator->read_elem_clamped(p.x + xi, p.y + yi, p.temp_color);
       /* Do not take the alpha channel into account. */
       const float delta_color = (fabsf(p.determ_reference_color[0] - p.temp_color[0]) +
                                  fabsf(p.determ_reference_color[1] - p.temp_color[1]) +
                                  fabsf(p.determ_reference_color[2] - p.temp_color[2]));
       if (delta_color < p.sigma_color) {
         /* Add this to the blur. */
-        p.input_color->read(p.temp_color, xi, yi);
+        p.input_color->read_elem_clamped(p.x + xi, p.y + yi, p.temp_color);
         add_v4_v4(p.out, p.temp_color);
         blur_divider += 1.0f;
       }
@@ -161,22 +157,17 @@ void BilateralBlurOperation::update_memory_buffer_partial(MemoryBuffer *output,
   PixelCursor p = {};
   p.step = QualityStepHelper::get_step();
   p.sigma_color = data_->sigma_color;
+  p.radius = radius_;
   p.input_color = inputs[0];
   p.input_determinator = inputs[1];
-  const float space = space_;
   for (int y = area.ymin; y < area.ymax; y++) {
     p.out = output->get_elem(area.xmin, y);
-    /* This will be used as the reference color for the determinator. */
-    p.determ_reference_color = p.input_determinator->get_elem(area.xmin, y);
-    p.min_y = floor(y - space);
-    p.max_y = ceil(y + space);
+    p.y = y;
     for (int x = area.xmin; x < area.xmax; x++) {
-      p.min_x = floor(x - space);
-      p.max_x = ceil(x + space);
-
+      p.x = x;
+      /* This will be used as the reference color for the determinator. */
+      p.determ_reference_color = p.input_determinator->get_elem(x, y);
       blur_pixel(p);
-
-      p.determ_reference_color += p.input_determinator->elem_stride;
       p.out += output->elem_stride;
     }
   }

source/blender/compositor/operations/COM_BilateralBlurOperation.h

@@ -4,6 +4,8 @@
 
 #pragma once
 
+#include "BLI_math_base.hh"
+
 #include "COM_MultiThreadedOperation.h"
 #include "COM_QualityStepHelper.h"
 
@@ -14,7 +16,7 @@ class BilateralBlurOperation : public MultiThreadedOperation, public QualityStep
   SocketReader *input_color_program_;
   SocketReader *input_determinator_program_;
   NodeBilateralBlurData *data_;
-  float space_;
+  int radius_;
 
  public:
   BilateralBlurOperation();
@@ -41,7 +43,7 @@ class BilateralBlurOperation : public MultiThreadedOperation, public QualityStep
   void set_data(NodeBilateralBlurData *data)
   {
     data_ = data;
-    space_ = data->sigma_space + data->iter;
+    radius_ = int(math::ceil(data->sigma_space + data->iter));
   }
 
   void get_area_of_interest(int input_idx, const rcti &output_area, rcti &r_input_area) override;