draw_if_possible Template Function Implementation for Image in C++

Question

This is a follow-up question for Midpoint Circle Algorithm Implementation for Image in C++, draw_circle Template Function Implementation for Image in C++, SIFT Keypoint Detection for Image in C++, difference_of_gaussian Template Function Implementation for Image in C++, conv2 Template Function Implementation for Image in C++ and imgaussfilt Template Function Implementation for Image in C++. To fix the issue mentioned in Rainer P.'s answer, I am trying to modify draw_circle template function and implement draw_if_possible template function in this post.

The experimental implementation

• draw_if_possible template function implementation (in file image_operations.h)

  namespace TinyDIP
  {
      //  draw_if_possible template function implementation
      template<typename ElementT>
      constexpr static void draw_if_possible(
          Image<ElementT>& input,
          ElementT draw_value,
          std::ptrdiff_t x,
          std::ptrdiff_t y
      )
      {
          if (x < 0 || x >= input.getWidth() ||
              y < 0 || y >= input.getHeight())
          {
              return;
          }
          input.at_without_boundary_check(x, y) = draw_value;
          return;
      }
  }
  

• draw_circle template function implementation (in file image_operations.h)

  namespace TinyDIP
  {
      //  draw_circle template function implementation
      //  https://codereview.stackexchange.com/q/293417/231235
      //  Test: https://godbolt.org/z/7zKfhG3x9
      template<typename ElementT>
      constexpr static auto draw_circle(
          const Image<ElementT>& input,
          std::tuple<std::size_t, std::size_t> central_point,
          std::size_t radius = 2,
          ElementT draw_value = ElementT{}
      )
      {
          if (input.getDimensionality() != 2)
          {
              throw std::runtime_error("Unsupported dimension!");
          }
          auto point_x = std::get<0>(central_point);
          auto point_y = std::get<1>(central_point);
          auto output = input;
          auto height = input.getHeight();
          auto width = input.getWidth();
          if (radius <= 0)
          {
              // early out avoids y going negative in loop
              return output;
          }
          for (std::ptrdiff_t x = 0, y = radius; x <= y; x++)
          {
              // try to decrement y, then accept or revert
              y--;
              if (x * x + y * y < radius * radius)
              {
                  y++;
              }
              // do nothing if out of bounds, otherwise draw
              draw_if_possible(output, draw_value, point_x + x, point_y + y);
              draw_if_possible(output, draw_value, point_x - x, point_y + y);
              draw_if_possible(output, draw_value, point_x + x, point_y - y);
              draw_if_possible(output, draw_value, point_x - x, point_y - y);
              draw_if_possible(output, draw_value, point_x + y, point_y + x);
              draw_if_possible(output, draw_value, point_x - y, point_y + x);
              draw_if_possible(output, draw_value, point_x + y, point_y - x);
              draw_if_possible(output, draw_value, point_x - y, point_y - x);
          }
          return output;
      }
  }
  

The example output:

The usage of draw_circle template function:

int main()
{
    auto start = std::chrono::system_clock::now();

    //  Read image file
    std::string file_path = "InputImages/1";
    auto bmp1 = TinyDIP::bmp_read(file_path.c_str(), false);

    //  Ascend image size
    bmp1 = copyResizeBicubic(bmp1, bmp1.getWidth() * 2, bmp1.getHeight() * 2);

    //  Get value plane
    auto v_plane = TinyDIP::getVplane(TinyDIP::rgb2hsv(bmp1));

    //  Get SIFT keypoints
    auto SIFT_keypoints = TinyDIP::SIFT_impl::get_potential_keypoint(v_plane);
    std::cout << "SIFT_keypoints = " << SIFT_keypoints.size() << "\n";

    //  Draw SIFT keypoints
    bmp1 = TinyDIP::draw_points(bmp1, SIFT_keypoints);
    for (auto&& each_SIFT_keypoint : SIFT_keypoints)
    {
        auto orientation_histogram = TinyDIP::SIFT_impl::get_orientation_histogram(v_plane, each_SIFT_keypoint);
        RGB rgb{ 255, 255, 255 };
        bmp1 = TinyDIP::draw_circle(bmp1, each_SIFT_keypoint, TinyDIP::recursive_max(orientation_histogram), rgb);
    }

    //  Save file
    TinyDIP::bmp_write("test20240829", bmp1);

    auto end = std::chrono::system_clock::now();
    std::chrono::duration<double> elapsed_seconds = end - start;
    std::time_t end_time = std::chrono::system_clock::to_time_t(end);
    if (elapsed_seconds.count() != 1)
    {
        std::cout << "Computation finished at " << std::ctime(&end_time) << "elapsed time: " << elapsed_seconds.count() << "seconds.\n";
    }
    else
    {
        std::cout << "Computation finished at " << std::ctime(&end_time) << "elapsed time: " << elapsed_seconds.count() << "second.\n";
    }
    
    return EXIT_SUCCESS;
}

TinyDIP on GitHub

All suggestions are welcome.

The summary information:

• Which question it is a follow-up to?

  Midpoint Circle Algorithm Implementation for Image in C++,

  draw_circle Template Function Implementation for Image in C++,

  SIFT Keypoint Detection for Image in C++,

  difference_of_gaussian Template Function Implementation for Image in C++,

  conv2 Template Function Implementation for Image in C++ and

  imgaussfilt Template Function Implementation for Image in C++

• What changes has been made in the code since last question?

  I am trying to modify draw_circle template function and implement draw_if_possible template function in this post.

• Why a new review is being asked for?

  Please review the implementation of draw_if_possible and draw_circle template functions and its tests.

Answer 1

Why not make this a member function of Image?

Instead of having to write:

draw_if_possible(output, draw_value, point_x + x, point_y + y);

Consider making it possible (pun not intended) to write:

output.set(point_x + x, point_y + y, draw_value);

Not only is it less typing for the caller, it also avoids some annoying problems with type deduction. Consider writing:

TinyDIP::Image<float> image(1, 1);
TinyDIP::draw_if_possible(image, 42, 0, 0);     // compile error!
TinyDIP::draw_if_possible(image, 3.1415, 0, 0); // compile error!

If it was a member function, Image::set() wouldn't be a template, and there would be no problem deducing the type of draw_value.

What about images that are not 2D?

Your TinyDIP::Image supports any number of dimensions, but draw_if_possible() only works for two-dimensional images. In fact, due to a total lack of checking in at_without_boundary_check(), it might crash if you try to call it on one-dimensional images.

Make sure you handle an arbitrary amount of coordinates. Perhaps even better would be to make the number of dimensions a template parameter of TinyDIP::Image, so it can be checked at compile time.

Use std::size_t for the coordinates

I get why you used std::ptrdiff_t for x and y, but it is inconsistent with the way you pass coordinates to any member function of Image. What if one dimension of the image is larger than can be stored in a std::ptrdiff_t?

The drawback of using std::size_t is that you might need to explicitly cast the std::ptrdiff_ts used in draw_circle() to std::size_t, which is annoying. Another solution would be to make the coordinate types template parameters, and then use std::cmp_less() and std::cmp_greater_equal() to compare them against the size of the image:

template<typename ElementT, typename... CoordinateTs>
constexpr static void draw_if_possible(
    Image<ElementT>& input,
    ElementT draw_value,
    CoordinateTs coordinates...)
)
{
    if ((std::cmp_less(coordinates, 0) || ...) ||
        (std::cmp_greater_equal(coordinates, /* something */) || ...))
    {
        return;
    }
    input.at_without_boundary_check(coordinates...) = draw_value;
}

It's not optimal for circle drawing

With your code you are checking every pixel you draw against the image size. However, you can be more efficient by doing the checks in draw_circle() itself. For example, you can already quickly check if the circle lies completely inside or completely outside the image. In the former case you can draw without per-pixel checks, in the latter case you can skip drawing any pixel. But even for circles that are only partially inside the image there are various ways you can do better.