From c7b99c7b87f30847170eb416c335229203f2d6a2 Mon Sep 17 00:00:00 2001
From: Kip Cole <kipcole9@gmail.com>
Date: Mon, 15 Jul 2024 09:26:55 +1000
Subject: [PATCH] Round get_pixel/3 values when the band format is integer

---
 CHANGELOG.md              |  10 +++
 lib/image.ex              |  73 ++++++++++++---------
 lib/image/draw.ex         | 134 ++++++++++++++++----------------------
 lib/image/nx.ex           |   3 +-
 lib/image/options/draw.ex |  84 ++++++++++++++----------
 mix.exs                   |   2 +-
 test/color_test.exs       |   5 +-
 test/flatten_test.exs     |   3 +-
 test/image_draw_test.exs  |  44 ++++++++-----
 test/image_test.exs       |   2 +-
 10 files changed, 195 insertions(+), 165 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index aace3e0..4e8d5c1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,15 @@
 # Changelog
 
+## Image 0.53.1
+
+This is the changelog for Image version 0.53.1 released on July 15th, 2024.  For older changelogs please consult the release tag on [GitHub](https://github.com/elixir-image/image/tags)
+
+### Bug Fixes
+
+* Fix typespecs in `Image.Draw`, improve tests and clarify docs. In particular, document that the function passed to `Image.mutate/2` *must* return either `:ok` or `{:ok, term}`.
+
+* Fix `Image.get_pixel/3` to ensure only integer values are returned when the image band format is integer. This is required because the underlying `Vix.Vips.Operation.getpoint/3` always returns floats.
+
 ## Image 0.53.0
 
 This is the changelog for Image version 0.53.0 released on July 14th, 2024.  For older changelogs please consult the release tag on [GitHub](https://github.com/elixir-image/image/tags)
diff --git a/lib/image.ex b/lib/image.ex
index 327736e..954e532 100644
--- a/lib/image.ex
+++ b/lib/image.ex
@@ -5246,7 +5246,8 @@ defmodule Image do
   """
   @doc subject: "Operation", since: "0.23.0"
 
-  @spec flatten(image :: Vimage.t(), options :: Keyword.t()) :: {:ok, Vimage.t()} | {:error, error_message()}
+  @spec flatten(image :: Vimage.t(), options :: Keyword.t()) ::
+          {:ok, Vimage.t()} | {:error, error_message()}
   def flatten(%Vimage{} = image, options \\ []) do
     background_color = Keyword.get(options, :background_color, :black)
 
@@ -6941,7 +6942,7 @@ defmodule Image do
     delta_e(color_1, color_2, version)
   end
 
-  def delta_e(color_1, color_2, version)  do
+  def delta_e(color_1, color_2, version) do
     with {:ok, version} <- validate_delta_e_version(version),
          {:ok, color_1} <- Color.validate_color(color_1),
          {:ok, color_2} <- Color.validate_color(color_2) do
@@ -7025,9 +7026,8 @@ defmodule Image do
 
   defp validate_delta_e_version(version) do
     {:error,
-        "Invalid delta_e version #{inspect version}. " <>
-        "Version must be one of #{inspect @delta_e_versions}"
-    }
+     "Invalid delta_e version #{inspect(version)}. " <>
+       "Version must be one of #{inspect(@delta_e_versions)}"}
   end
 
   if Code.ensure_loaded?(Scholar.Cluster.KMeans) do
@@ -7122,7 +7122,7 @@ defmodule Image do
     @doc subject: "Clusters", since: "0.49.0"
 
     @spec k_means(image :: Vimage.t(), options :: Keyword.t()) ::
-      {:ok, list(Color.t())} | {:error, error_message()}
+            {:ok, list(Color.t())} | {:error, error_message()}
 
     def k_means(%Vimage{} = image, options \\ []) do
       options = Keyword.put_new(options, :num_clusters, @default_clusters)
@@ -7229,7 +7229,7 @@ defmodule Image do
     @doc subject: "Clusters", since: "0.49.0"
 
     @spec k_means!(image :: Vimage.t(), options :: Keyword.t()) ::
-      list(Color.t()) | no_return()
+            list(Color.t()) | no_return()
 
     def k_means!(%Vimage{} = image, options \\ []) do
       case k_means(image, options) do
@@ -7363,6 +7363,9 @@ defmodule Image do
   the length of the list is equal to the number
   of bands in the image.
 
+  If the colorspace of the image is `:srgb` then
+  the values are rounded.
+
   ### Arguments
 
   * `image` is any `t:Vix.Vips.Image.t/0`.
@@ -7388,8 +7391,18 @@ defmodule Image do
           {:ok, Color.rgb_color()} | {:error, error_message()}
 
   def get_pixel(%Vimage{} = image, x, y) do
+    band_format = Image.band_format(image)
+
     with {:ok, values} <- Operation.getpoint(image, x, y) do
-      {:ok, Enum.map(values, &round/1)}
+      values =
+        case band_format do
+          {:u, _} ->
+            Enum.map(values, &round/1)
+          _other ->
+            values
+        end
+
+      {:ok, values}
     end
   end
 
@@ -7439,7 +7452,7 @@ defmodule Image do
   Mutations, like those functions in the
   `Image.Draw`, module are operations on
   a *copy* of the base image and operations
-  are serialized through a genserver in order
+  are serialized through a gen_server in order
   to maintain thread safety.
 
   In order to perform multiple mutations without
@@ -7461,15 +7474,13 @@ defmodule Image do
   process is ended and a normal `t:Vix.Vips.Image.t/0`
   is returned.
 
-  This function is a convenience wrapper
-  around `Vix.Vips.Image.mutate/2`.
-
   ### Arguments
 
   * `image` is any `t:Vix.Vips.Image.t/0`.
 
   * `fun` is any 1-arity function that receives
-    a `t:Vix.Vips.MutableImage.t/0` parameter.
+    a `t:Vix.Vips.MutableImage.t/0` parameter. This function
+    *must* return either `:ok` or `{:ok, term}`.
 
   ### Returns
 
@@ -7477,21 +7488,21 @@ defmodule Image do
 
   * `{:error, reason}`
 
+  ### Notes
+
+  The image is copied and operations are serialized behind a gen_server.
+  Only one copy is made but all operations will be serialized behind the
+  gen_server. When the function returns, the gen_server is broken down and
+  the underlying mutated `t:Vix.Vips.Image.t/0` is returned.
+
   ### Example
 
-        # The image is copied and operations
-        # are serialized behind a genserver.
-        # Only one copy is made but all operations
-        # will be serialized behind a genserver.
-        # When the function returns the genserver
-        # is broken down and the underlying
-        # mutated `t:Vix.Vips.Image.t/0` is returned.
-
-        Image.mutate image, fn mutable_image ->
-          mutable_image
-          |> Image.Draw.rect!(0, 0, 10, 10, color: :red)
-          |> Image.Draw.rect!(10, 10, 20, 20, color: :green)
-        end
+      iex> {:ok, image} = Image.open("./test/support/images/puppy.webp")
+      iex> {:ok, _mutated_copy} =
+      ...>   Image.mutate(image, fn mut_image ->
+      ...>     cx = cy = div(Image.height(image), 2)
+      ...>     {:ok, _image} = Image.Draw.circle(mut_image, cx, cy, 100, color: :green)
+      ...>   end)
 
   """
   @doc subject: "Operation", since: "0.7.0"
@@ -7500,7 +7511,11 @@ defmodule Image do
           {:ok, Vimage.t()} | {:error, error_message()}
 
   def mutate(%Vimage{} = image, fun) when is_function(fun, 1) do
-    Vimage.mutate(image, fun)
+    case Vimage.mutate(image, fun) do
+      {:error, reason} -> {:error, reason}
+      {:ok, {image, _other}} -> {:ok, image}
+      {:ok, image} -> {:ok, image}
+    end
   end
 
   @doc """
@@ -10091,7 +10106,7 @@ defmodule Image do
   @doc subject: "Split and join", since: "0.53.0"
 
   @spec join_bands(image_list :: [Vimage.t()]) ::
-    {:ok, Vimage.t()} | {:error, error_message()}
+          {:ok, Vimage.t()} | {:error, error_message()}
 
   def join_bands(bands) when is_list(bands) do
     Operation.bandjoin(bands)
@@ -10119,7 +10134,7 @@ defmodule Image do
   @doc subject: "Split and join", since: "0.53.0"
 
   @spec join_bands!(image_list :: [Vimage.t()]) ::
-    Vimage.t() | no_return()
+          Vimage.t() | no_return()
 
   def join_bands!(bands) when is_list(bands) do
     case join_bands(bands) do
diff --git a/lib/image/draw.ex b/lib/image/draw.ex
index 2003481..7978925 100644
--- a/lib/image/draw.ex
+++ b/lib/image/draw.ex
@@ -71,7 +71,12 @@ defmodule Image.Draw do
   """
   @doc since: "0.7.0"
 
-  @spec point(Vimage.t(), non_neg_integer(), non_neg_integer(), Options.Draw.point()) ::
+  @spec point(
+          Vimage.t() | MutableImage.t(),
+          non_neg_integer(),
+          non_neg_integer(),
+          Options.Draw.point()
+        ) ::
           {:ok, Vimage.t()} | {:error, Image.error_message()}
 
   def point(image, left, top, options \\ [])
@@ -87,9 +92,6 @@ defmodule Image.Draw do
     end
   end
 
-  @spec point(MutableImage.t(), non_neg_integer(), non_neg_integer(), Options.Draw.point()) ::
-          {:ok, MutableImage.t()} | {:error, Image.error_message()}
-
   def point(%MutableImage{} = image, left, top, options) when is_point(left, top) do
     with {:ok, options} <- Options.Draw.validate_options(:point, options) do
       color = maybe_add_alpha(image, options.color)
@@ -239,18 +241,22 @@ defmodule Image.Draw do
   # one filled rectangle for each of the four sides.
 
   defp rect(%Vimage{} = image, left, top, width, height, color, stroke_width, fill) do
-    Vimage.mutate(image, fn image ->
+    Image.mutate(image, fn image ->
       do_rect(image, left, top, width, height, color, stroke_width, fill)
     end)
   end
 
   defp rect(%MutableImage{} = image, left, top, width, height, color, stroke_width, fill) do
-    do_rect(image, left, top, width, height, color, stroke_width, fill)
+    :ok = do_rect(image, left, top, width, height, color, stroke_width, fill)
+    {:ok, image}
   end
 
+  # do_rect/8 operates within the mutation closure. Its results are
+  # ignored, just return the expected :ok.
+
   defp do_rect(%MutableImage{} = image, left, top, width, height, color, stroke_width, fill)
        when fill == true or stroke_width == 1 do
-    MutableOperation.draw_rect(image, color, left, top, width, height, fill: fill)
+    :ok = MutableOperation.draw_rect(image, color, left, top, width, height, fill: fill)
   end
 
   defp do_rect(%MutableImage{} = image, left, top, width, height, color, stroke_width, _fill) do
@@ -428,13 +434,14 @@ defmodule Image.Draw do
   # wider stroke width.
 
   defp circle(%Vimage{} = image, cx, cy, radius, color, stroke_width, fill) do
-    Vimage.mutate(image, fn image ->
+    Image.mutate(image, fn image ->
       do_circle(image, cx, cy, radius, color, stroke_width, fill)
     end)
   end
 
   defp circle(%MutableImage{} = image, cx, cy, radius, color, stroke_width, fill) do
-    do_circle(image, cx, cy, radius, color, stroke_width, fill)
+    :ok = do_circle(image, cx, cy, radius, color, stroke_width, fill)
+    {:ok, image}
   end
 
   defp do_circle(%MutableImage{} = image, cx, cy, radius, color, stroke_width, fill)
@@ -572,7 +579,7 @@ defmodule Image.Draw do
   @doc since: "0.7.0"
 
   @spec line(
-          Vimage.t(),
+          Vimage.t() | MutableImage.t(),
           non_neg_integer(),
           non_neg_integer(),
           non_neg_integer(),
@@ -587,31 +594,20 @@ defmodule Image.Draw do
       when is_integer(x1) and is_integer(y1) and x1 >= 0 and y1 >= 0 and
              is_integer(x2) and is_integer(y2) and x2 >= 0 and y2 >= 0 do
     with {:ok, options} <- Options.Draw.validate_options(:line, options) do
-      color = maybe_add_alpha(image, options.color)
-
-      Vimage.mutate(image, fn mut_img ->
-        MutableOperation.draw_line(mut_img, color, x1, y1, x2, y2)
+      Image.mutate(image, fn mut_img ->
+        line(mut_img, x1, y1, x2, y2, options)
       end)
     end
     |> maybe_wrap()
   end
 
-  @spec line(
-          MutableImage.t(),
-          non_neg_integer(),
-          non_neg_integer(),
-          non_neg_integer(),
-          non_neg_integer(),
-          Options.Draw.line()
-        ) ::
-          {:ok, MutableImage.t()} | {:error, Image.error_message()}
-
   def line(%MutableImage{} = image, x1, y1, x2, y2, options)
       when is_integer(x1) and is_integer(y1) and x1 >= 0 and y1 >= 0 and
              is_integer(x2) and is_integer(y2) and x2 >= 0 and y2 >= 0 do
     with {:ok, options} <- Options.Draw.validate_options(:line, options) do
       color = maybe_add_alpha(image, options.color)
-      MutableOperation.draw_line(image, color, x1, y1, x2, y2)
+      :ok = MutableOperation.draw_line(image, color, x1, y1, x2, y2)
+      {:ok, image}
     end
     |> maybe_wrap()
   end
@@ -664,7 +660,7 @@ defmodule Image.Draw do
   @doc since: "0.17.0"
 
   @spec line!(
-          Vimage.t(),
+          Vimage.t() | MutableImage.t(),
           non_neg_integer(),
           non_neg_integer(),
           non_neg_integer(),
@@ -730,34 +726,30 @@ defmodule Image.Draw do
   """
   @doc since: "0.7.0"
 
-  @spec image(Vimage.t(), Vimage.t(), non_neg_integer(), non_neg_integer(), Options.Draw.image()) ::
+  @spec image(
+          Vimage.t() | MutableImage.t(),
+          Vimage.t(),
+          non_neg_integer(),
+          non_neg_integer(),
+          Options.Draw.image()
+        ) ::
           {:ok, Vimage.t()} | {:error, Image.error_message()}
 
   def image(image, sub_image, top, left, options \\ [])
 
   def image(%Vimage{} = image, %Vimage{} = sub_image, top, left, options)
       when is_integer(top) and is_integer(left) and left >= 0 and top >= 0 do
-    with {:ok, options} <- Options.Draw.validate_options(:image, options) do
-      Vimage.mutate(image, fn mut_img ->
-        MutableOperation.draw_image(mut_img, sub_image, top, left, Map.to_list(options))
-      end)
-    end
+    Image.mutate(image, fn mut_img ->
+      image(mut_img, sub_image, top, left, options)
+    end)
     |> maybe_wrap()
   end
 
-  @spec image(
-          MutableImage.t(),
-          Vimage.t(),
-          non_neg_integer(),
-          non_neg_integer(),
-          Options.Draw.image()
-        ) ::
-          {:ok, Vimage.t()} | {:error, Image.error_message()}
-
   def image(%MutableImage{} = image, %Vimage{} = sub_image, top, left, options)
       when is_integer(top) and is_integer(left) and top >= 0 and left >= 0 do
     with {:ok, options} <- Options.Draw.validate_options(:image, options) do
-      MutableOperation.draw_image(image, sub_image, top, left, Map.to_list(options))
+      :ok = MutableOperation.draw_image(image, sub_image, top, left, Map.to_list(options))
+      {:ok, image}
     end
     |> maybe_wrap()
   end
@@ -882,7 +874,7 @@ defmodule Image.Draw do
     0-based offsets from the top and left location respectively
     of the flood area.
 
-  * or `{:error, reason}`
+  * or `{:error, reason}`.
 
   """
   @doc since: "0.7.0"
@@ -894,7 +886,7 @@ defmodule Image.Draw do
           Options.Draw.flood()
         ) ::
           {:ok,
-           {Vimage.t(), [height: integer(), width: integer(), top: integer(), left: integer()]}}
+           {Vimage.t(), %{height: integer(), width: integer(), top: integer(), left: integer()}}}
           | {:error, Image.error_message()}
 
   def flood(%image_type{} = image, left, top, options \\ [])
@@ -907,7 +899,7 @@ defmodule Image.Draw do
   end
 
   defp flood(%Vimage{} = image, left, top, color, equal) do
-    Vimage.mutate(image, fn image ->
+    Image.mutate(image, fn image ->
       flood(image, left, top, color, equal)
     end)
   end
@@ -974,7 +966,12 @@ defmodule Image.Draw do
   """
   @doc since: "0.24.0"
 
-  @spec flood!(Vimage.t(), non_neg_integer(), non_neg_integer(), Options.Draw.flood()) ::
+  @spec flood!(
+          Vimage.t() | MutableImage.t(),
+          non_neg_integer(),
+          non_neg_integer(),
+          Options.Draw.flood()
+        ) ::
           Vimage.t() | no_return()
 
   def flood!(%Vimage{} = image, left, top, options \\ []) do
@@ -993,7 +990,13 @@ defmodule Image.Draw do
   """
   @doc since: "0.7.0"
 
-  @spec mask(Vimage.t(), Vimage.t(), non_neg_integer(), non_neg_integer(), Options.Draw.mask()) ::
+  @spec mask(
+          Vimage.t() | MutableImage.t(),
+          Vimage.t(),
+          non_neg_integer(),
+          non_neg_integer(),
+          Options.Draw.mask()
+        ) ::
           {:ok,
            {Vimage.t(), [height: integer(), width: integer(), top: integer(), left: integer()]}}
           | {:error, Image.error_message()}
@@ -1005,29 +1008,19 @@ defmodule Image.Draw do
     with {:ok, options} <- Options.Draw.validate_options(:mask, options) do
       color = maybe_add_alpha(image, options.color)
 
-      Vimage.mutate(image, fn mut_img ->
+      Image.mutate(image, fn mut_img ->
         MutableOperation.draw_mask(mut_img, color, mask, x, y)
       end)
     end
     |> maybe_wrap()
   end
 
-  @spec mask(
-          MutableImage.t(),
-          Vimage.t(),
-          non_neg_integer(),
-          non_neg_integer(),
-          Options.Draw.mask()
-        ) ::
-          {:ok,
-           {Vimage.t(), [height: integer(), width: integer(), top: integer(), left: integer()]}}
-          | {:error, Image.error_message()}
-
   def mask(%MutableImage{} = image, %Vimage{} = mask, x, y, options)
       when is_integer(x) and is_integer(y) and x >= 0 and y >= 0 do
     with {:ok, options} <- Options.Draw.validate_options(:mask, options) do
       color = maybe_add_alpha(image, options.color)
-      MutableOperation.draw_mask(image, color, mask, x, y)
+      :ok = MutableOperation.draw_mask(image, color, mask, x, y)
+      {:ok, image}
     end
     |> maybe_wrap()
   end
@@ -1042,7 +1035,7 @@ defmodule Image.Draw do
   @doc since: "0.7.0"
 
   @spec smudge(
-          Vimage.t(),
+          Vimage.t() | MutableImage.t(),
           non_neg_integer(),
           non_neg_integer(),
           pos_integer(),
@@ -1057,28 +1050,19 @@ defmodule Image.Draw do
       when is_integer(left) and is_integer(top) and left >= 0 and top >= 0
       when is_integer(width) and is_integer(height) and width > 0 and height > 0 do
     with {:ok, _options} <- Options.Draw.validate_options(:smudge, options) do
-      Vimage.mutate(image, fn mut_img ->
+      Image.mutate(image, fn mut_img ->
         MutableOperation.draw_smudge(mut_img, left, top, width, height)
       end)
     end
     |> maybe_wrap()
   end
 
-  @spec smudge(
-          MutableImage.t(),
-          non_neg_integer(),
-          non_neg_integer(),
-          pos_integer(),
-          pos_integer(),
-          Options.Draw.smudge()
-        ) ::
-          :ok | {:error, Image.error_message()}
-
   def smudge(%MutableImage{} = image, left, top, width, height, options)
       when is_integer(left) and is_integer(top) and left >= 0 and top >= 0
       when is_integer(width) and is_integer(height) and width > 0 and height > 0 do
     with {:ok, _options} <- Options.Draw.validate_options(:smudge, options) do
-      MutableOperation.draw_smudge(image, left, top, width, height)
+      :ok = MutableOperation.draw_smudge(image, left, top, width, height)
+      {:ok, image}
     end
     |> maybe_wrap()
   end
@@ -1115,10 +1099,6 @@ defmodule Image.Draw do
     Vimage.has_alpha?(image)
   end
 
-  defp maybe_wrap({:ok, {image, {box}}}) when is_list(box) do
-    {:ok, {image, box}}
-  end
-
   defp maybe_wrap({:ok, result}) do
     {:ok, result}
   end
diff --git a/lib/image/nx.ex b/lib/image/nx.ex
index 7345696..bf9e886 100644
--- a/lib/image/nx.ex
+++ b/lib/image/nx.ex
@@ -61,6 +61,5 @@ if Code.ensure_loaded?(Nx) do
       mask = logical_or(tensor, tensor)
       masked_select(tensor, mask)
     end
-
   end
-end
\ No newline at end of file
+end
diff --git a/lib/image/options/draw.ex b/lib/image/options/draw.ex
index 9bee9c0..768eb0f 100644
--- a/lib/image/options/draw.ex
+++ b/lib/image/options/draw.ex
@@ -8,39 +8,53 @@ defmodule Image.Options.Draw do
   alias Image.Color
   alias Image.CombineMode
 
-  @type circle :: [
-          {:fill, boolean()}
-          | {:color, Color.t()}
-        ]
-
-  @type rect :: [
-          {:fill, boolean()}
-          | {:color, Color.t()}
-          | {:stroke_width, pos_integer()}
-        ]
-
-  @type point :: [
-          {:color, Color.t()}
-        ]
-
-  @type flood :: [
-          {:equal, boolean()}
-          | {:color, Color.t()}
-        ]
-
-  @type mask :: [
-          {:color, Color.t()}
-        ]
-
-  @type line :: [
-          {:color, Color.t()}
-        ]
-
-  @type smudge :: []
-
-  @type image :: [
-          {:mode, CombineMode.t()}
-        ]
+  @type circle ::
+          [
+            {:fill, boolean()}
+            | {:color, Color.t()}
+          ]
+          | map()
+
+  @type rect ::
+          [
+            {:fill, boolean()}
+            | {:color, Color.t()}
+            | {:stroke_width, pos_integer()}
+          ]
+          | map()
+
+  @type point ::
+          [
+            {:color, Color.t()}
+          ]
+          | map()
+
+  @type flood ::
+          [
+            {:equal, boolean()}
+            | {:color, Color.t()}
+          ]
+          | map()
+
+  @type mask ::
+          [
+            {:color, Color.t()}
+          ]
+          | map()
+
+  @type line ::
+          [
+            {:color, Color.t()}
+          ]
+          | map()
+
+  @type smudge :: [] | map()
+
+  @type image ::
+          [
+            {:mode, CombineMode.t()}
+          ]
+          | map()
 
   @doc false
   def default_options(:circle) do
@@ -105,6 +119,10 @@ defmodule Image.Options.Draw do
   Validate the options for `Image.Draw`.
 
   """
+  def validate_options(_type, %{} = options) do
+    {:ok, options}
+  end
+
   def validate_options(type, options) do
     options = Keyword.merge(default_options(type), options)
 
diff --git a/mix.exs b/mix.exs
index 739d6ff..4db988e 100644
--- a/mix.exs
+++ b/mix.exs
@@ -1,7 +1,7 @@
 defmodule Image.MixProject do
   use Mix.Project
 
-  @version "0.53.0"
+  @version "0.53.1"
 
   @app_name "image"
 
diff --git a/test/color_test.exs b/test/color_test.exs
index 948ef1d..46578ce 100644
--- a/test/color_test.exs
+++ b/test/color_test.exs
@@ -2,7 +2,6 @@ defmodule Image.ColorTest do
   use ExUnit.Case, async: true
 
   test "Sort colors" do
-    assert [[1, 5, 1], [1, 3, 2]] = Image.Color.sort([[1,5,1], [1,3,2]])
-
+    assert [[1, 5, 1], [1, 3, 2]] = Image.Color.sort([[1, 5, 1], [1, 3, 2]])
   end
-end
\ No newline at end of file
+end
diff --git a/test/flatten_test.exs b/test/flatten_test.exs
index 7e810ed..b8b61d1 100644
--- a/test/flatten_test.exs
+++ b/test/flatten_test.exs
@@ -27,5 +27,4 @@ defmodule Image.FlattenTest do
 
     assert_images_equal(result, validate_path)
   end
-
-end
\ No newline at end of file
+end
diff --git a/test/image_draw_test.exs b/test/image_draw_test.exs
index 2d1fa34..d95d79f 100644
--- a/test/image_draw_test.exs
+++ b/test/image_draw_test.exs
@@ -1,33 +1,43 @@
 defmodule Image.Draw.Test do
   use ExUnit.Case, async: true
-  alias Vix.Vips.Image, as: Vimage
   import Image.TestSupport
 
-  test "drawing a circle on a white image" do
+  test "mutating draw a rectangle on a white image" do
+    {:ok, image} = Vix.Vips.Operation.black!(500, 500, bands: 3) |> Vix.Vips.Operation.invert()
+
+    assert {:ok, _image} =
+             Image.mutate(image, fn mut_img ->
+               {:ok, _} = Image.Draw.rect(mut_img, 10, 10, 100, 100, color: :green, fill: true)
+             end)
+  end
+
+  test "mutating draw a circle on a white image" do
     {:ok, image} = Vix.Vips.Operation.black!(500, 500, bands: 3) |> Vix.Vips.Operation.invert()
     cx = cy = div(Image.height(image), 2)
 
-    {:ok, image} =
-      Vimage.mutate(image, fn mut_img ->
-        :ok = Vix.Vips.MutableOperation.draw_circle(mut_img, [0, 200, 0], cx, cy, 100, fill: true)
-      end)
+    assert {:ok, _image} =
+             Image.mutate(image, fn mut_img ->
+               {:ok, _} = Image.Draw.circle(mut_img, cx, cy, 100, fill: true, color: [0, 200, 0])
+             end)
+  end
+
+  test "mutating draw a line on a white image" do
+    {:ok, image} = Vix.Vips.Operation.black!(500, 500, bands: 3) |> Vix.Vips.Operation.invert()
 
-    Image.write(image, "/Users/kip/Desktop/draw.jpg")
+    assert {:ok, _image} =
+             Image.mutate(image, fn mut_img ->
+               {:ok, _} = Image.Draw.line(mut_img, 0, 0, 499, 499, color: [0, 200, 0])
+             end)
   end
 
-  test "draw an image onto another image" do
+  test "mutating draw an image onto another image" do
     {:ok, image} = Vix.Vips.Operation.black(500, 500, bands: 4)
     {:ok, star} = Image.Shape.star(5, rotation: 90, fill_color: :green, stroke_color: :green)
 
-    {:ok, image} =
-      Vimage.mutate(image, fn mut_img ->
-        :ok =
-          Vix.Vips.MutableOperation.draw_image(mut_img, star, 100, 100,
-            mode: :VIPS_COMBINE_MODE_ADD
-          )
-      end)
-
-    Image.write(image, "/Users/kip/Desktop/draw2.png")
+    assert {:ok, _image} =
+             Image.mutate(image, fn mut_img ->
+               {:ok, _} = Image.Draw.image(mut_img, star, 100, 100, mode: :VIPS_COMBINE_MODE_ADD)
+             end)
   end
 
   test "draw a line" do
diff --git a/test/image_test.exs b/test/image_test.exs
index 1ae9817..50613e6 100644
--- a/test/image_test.exs
+++ b/test/image_test.exs
@@ -322,7 +322,7 @@ defmodule Image.Test do
   end
 
   test "Joining bands results in the same image" do
-    image = Image.open! "./test/support/images/Singapore-2016-09-5887.jpg"
+    image = Image.open!("./test/support/images/Singapore-2016-09-5887.jpg")
     bands = Image.split_bands(image)
     joined = Image.join_bands!(bands)
     assert {:ok, +0.0, _} = Image.compare(image, joined)