Fix Image.Pixel.to_pixel/3 returning gamma encoded colors and 0..255 greys for scRGB images

CHANGELOG.md

@@ -12,6 +12,10 @@
 
 ### Changed
 
+* **Breaking:** `Image.Pixel.to_pixel/3` returns linear light values for an `:scrgb` image. It previously returned gamma encoded sRGB, so a color drawn onto an scRGB image came out too bright and desaturated. This affects every function that resolves a color against an scRGB image. ([#230](https://github.com/elixir-image/image/pull/230))
+
+* **Breaking:** `Image.Pixel.to_pixel/3` returns relative luminance (CIE `Y`) for a single band `:scrgb` image. It previously returned a `0..255` integer, so `:white` resolved to `255` rather than `1.0`. ([#230](https://github.com/elixir-image/image/pull/230))
+
 * **Breaking:** `Image.average/1` and `Image.chroma_color/1` now return `{:ok, [number()]} | {:error, Image.Error.t()}` instead of a bare list on success. The previous success type was documented as `Pixel.t()` but was always a list of numbers. ([#219](https://github.com/elixir-image/image/pull/219))
 
 * **Breaking:** `Image.warp_perspective/4` and `Image.straighten_perspective/3` now preserve image alpha instead of always flattening, so the band count and pixels of the result may change. An omitted `:background` defers to libvips' fill rather than defaulting to `:black`, matching the other background-taking functions. Images without alpha are unaffected, since libvips fills those with black. ([#216](https://github.com/elixir-image/image/pull/216))

lib/image/pixel.ex

@@ -63,7 +63,7 @@ defmodule Image.Pixel do
     srgb: {Color.SRGB, :uchar_rgb},
     rgb: {Color.SRGB, :uchar_rgb},
     rgb16: {Color.SRGB, :ushort_rgb},
-    scrgb: {Color.SRGB, :float_rgb},
+    scrgb: {Color.RGB, :float_rgb},
     lab: {Color.Lab, :float_lab},
     labs: {Color.Lab, :short_lab},
     lch: {Color.LCHab, :float_lch},
@@ -139,12 +139,19 @@ defmodule Image.Pixel do
   * For float interpretations (`:scrgb`, `:lab`, `:lch`, etc.) the
     output is floats in the natural range of that space.
 
+  * `:scrgb` is linear light: mid grey `"#808080"` encodes as `0.216`,
+    not `0.502`.
+
   * The output band count matches `Vix.Vips.Image.bands/1` exactly.
     Alpha is appended when the image has an alpha band, and stripped
     when it does not.
 
-  * 1-band (`:bw`, `:grey16`) images receive a single luminance
-    channel computed from the perceptually-uniform `Color.Lab` `L*`.
+  * 1-band `:bw` and `:grey16` images receive a single luminance
+    channel computed from the perceptually-uniform `Color.Lab` `L*`,
+    as does any other interpretation that arrives with one band.
+
+  * 1-band `:scrgb` is the exception: it holds linear light, so it
+    receives relative luminance computed from `Color.XYZ` `Y`.
 
   ### Examples
 
@@ -258,7 +265,7 @@ defmodule Image.Pixel do
 
     with {:ok, source_struct} <- resolve(color),
          {:ok, {target_module, encoder}} <- target_for(interpretation, color_bands),
-         {:ok, converted} <- Color.convert(source_struct, target_module, intent: intent),
+         {:ok, converted} <- convert(source_struct, target_module, intent),
          {:ok, base_pixel} <- encode(encoder, converted),
          {:ok, alpha_value} <- alpha_for(encoder, explicit_alpha, source_struct, has_alpha) do
       {:ok, fit_bands(base_pixel, alpha_value, bands, has_alpha)}
@@ -498,12 +505,23 @@ defmodule Image.Pixel do
 
   defp resolve(other), do: Color.new(other)
 
+  # Color.RGB is the only target that needs a working space
+  # libvips scRGB is linear light on the sRGB primaries.
+  defp convert(source, Color.RGB, intent),
+    do: Color.convert(source, Color.RGB, :SRGB, intent: intent)
+
+  defp convert(source, target, intent),
+    do: Color.convert(source, target, intent: intent)
+
   # When the image has only one color channel (greyscale), force a
   # luma encoder regardless of the nominal interpretation. libvips
   # tags single-band images as :srgb / :rgb / :multiband fairly often
-  # so we can't rely on the interpretation atom alone.
+  # so we can't rely on the interpretation atom alone. The tag still
+  # picks the value range, which the band count cannot tell us.
+  defp target_for(:scrgb, 1), do: {:ok, {Color.RGB, :float_grey}}
+
   defp target_for(interpretation, 1)
-       when interpretation in [:srgb, :rgb, :multiband, :bw, :scrgb],
+       when interpretation in [:srgb, :rgb, :multiband, :bw],
        do: {:ok, {Color.SRGB, :uchar_grey}}
 
   defp target_for(interpretation, 1) when interpretation in [:rgb16, :grey16],
@@ -529,9 +547,15 @@ defmodule Image.Pixel do
   defp encode(:ushort_rgb, %Color.SRGB{r: r, g: g, b: b}),
     do: {:ok, [scale(r, 65_535), scale(g, 65_535), scale(b, 65_535)]}
 
-  defp encode(:float_rgb, %Color.SRGB{r: r, g: g, b: b}),
+  defp encode(:float_rgb, %Color.RGB{r: r, g: g, b: b}),
     do: {:ok, [r * 1.0, g * 1.0, b * 1.0]}
 
+  defp encode(:float_grey, %Color.RGB{} = rgb) do
+    with {:ok, %Color.XYZ{y: y}} <- Color.RGB.to_xyz(rgb) do
+      {:ok, [y * 1.0]}
+    end
+  end
+
   defp encode(:float_lab, %Color.Lab{l: l, a: a, b: b}),
     do: {:ok, [l * 1.0, a * 1.0, b * 1.0]}
 
@@ -611,6 +635,7 @@ defmodule Image.Pixel do
     :short_lab
   ]
   @alpha_max_65535 [:ushort_rgb, :ushort_grey]
+  @alpha_max_1 [:float_rgb, :float_grey]
 
   # The alpha band uses the same numeric type as the rest of the
   # interpretation: 0..255 for uchar, 0..65535 for ushort, 0.0..1.0
@@ -621,7 +646,7 @@ defmodule Image.Pixel do
     case encoder do
       e when e in @alpha_max_255 -> scale(alpha, 255)
       e when e in @alpha_max_65535 -> scale(alpha, 65_535)
-      :float_rgb -> alpha * 1.0
+      e when e in @alpha_max_1 -> alpha * 1.0
     end
   end
 

test/pixel_test.exs

@@ -205,6 +205,57 @@ defmodule Image.PixelTest do
     end
   end
 
+  describe "to_pixel/3 against an scRGB image" do
+    setup do
+      {:ok, image} = Image.new(2, 2, color: :black)
+      {:ok, image} = Image.to_colorspace(image, :scrgb)
+      {:ok, image: image}
+    end
+
+    test "round trips through libvips back to the requested sRGB color", %{image: image} do
+      colors = [
+        "#000000",
+        "#010101",
+        "#404040",
+        "#808080",
+        "#fefefe",
+        "#ffffff",
+        "#4080c0",
+        "#663399"
+      ]
+
+      for color <- colors do
+        {:ok, pixel} = Pixel.to_pixel(image, color)
+        {:ok, drawn} = Image.Draw.rect(image, 0, 0, 2, 2, color: pixel)
+        {:ok, srgb} = Image.to_colorspace(drawn, :srgb)
+
+        assert {:ok, actual} = Image.get_pixel(srgb, 0, 0)
+        assert {:ok, expected} = Pixel.to_srgb(color)
+        assert actual == expected, "#{inspect(color)} round tripped to #{inspect(actual)}"
+      end
+    end
+
+    test "a float list passes through unclamped", %{image: image} do
+      assert Pixel.to_pixel(image, [2.5, 0.0, -0.1]) == {:ok, [2.5, 0.0, -0.1]}
+    end
+  end
+
+  describe "to_pixel/3 against a single band scRGB image" do
+    setup do
+      {:ok, image} = Image.new(2, 2, color: :black)
+      {:ok, scrgb} = Image.to_colorspace(image, :scrgb)
+      {:ok, image: hd(Image.split_bands(scrgb))}
+    end
+
+    test "resolves to the color's CIE Y", %{image: image} do
+      for color <- [:white, :black, "#808080", :red, :lime, :blue, "#663399"] do
+        assert {:ok, [y]} = Pixel.to_pixel(image, color)
+        assert {:ok, %Color.XYZ{y: expected}} = Color.convert(color, Color.XYZ)
+        assert_in_delta y, expected, 0.0001, "#{inspect(color)} resolved to #{y}"
+      end
+    end
+  end
+
   describe "strip_alpha/2" do
     test "drops the last band of a full pixel on an alpha image" do
       {:ok, image} = Image.new(2, 2, color: [0, 0, 0, 255])