Fix and test Image#subimage, loading images from rects, and BLOB roun…

…dtrips (#674)

ext/gosu-ffi/gosu-ffi.def

@@ -213,6 +213,7 @@ Gosu_IF_TILEABLE_TOP
 Gosu_Image_create
 Gosu_Image_create_from_blob
 Gosu_Image_create_from_markup
+Gosu_Image_create_from_rect
 Gosu_Image_create_from_subimage
 Gosu_Image_create_from_text
 Gosu_Image_create_from_tiles

ffi/Gosu_FFI_internal.h

@@ -26,7 +26,7 @@ auto Gosu_translate_exceptions(Functor functor)
     }
 }
 
-// C-compatible wrapper structs for Gosu classes
+// C-compatible wrapper structs for Gosu classes. (No inheritance because of missing virtual dtors.)
 
 struct Gosu_Channel
 {
@@ -48,7 +48,7 @@ struct Gosu_Sample
     Gosu::Sample sample;
 };
 
-// Use inheritance where composition is not feasible
+// Use inheritance where composition is not feasible (because we want to compare pointers).
 
 struct Gosu_Song : Gosu::Song
 {

ffi/Gosu_Image.cpp

@@ -2,9 +2,18 @@
 #include <cstring> // for std::memcpy
 
 GOSU_FFI_API Gosu_Image* Gosu_Image_create(const char* filename, unsigned image_flags)
+{
+    return Gosu_translate_exceptions([=] { //
+        return new Gosu_Image { Gosu::Image(filename, image_flags) };
+    });
+}
+
+GOSU_FFI_API Gosu_Image* Gosu_Image_create_from_rect(const char* filename, //
+                                                int x, int y, int width, int height,
+                                                unsigned image_flags)
 {
     return Gosu_translate_exceptions([=] {
-        return new Gosu_Image{Gosu::Image{filename, image_flags}};
+        return new Gosu_Image { Gosu::Image(filename, { x, y, width, height }, image_flags) };
     });
 }
 
@@ -32,32 +41,35 @@ GOSU_FFI_API Gosu_Image* Gosu_Image_create_from_text(const char* text, const cha
     });
 }
 
-GOSU_FFI_API Gosu_Image* Gosu_Image_create_from_blob(void* blob, int byte_count, int columns,
-                                                     int rows, unsigned image_flags)
+GOSU_FFI_API Gosu_Image* Gosu_Image_create_from_blob(void* blob, size_t byte_count, //
+                                                     int columns, int rows, //
+                                                     int x, int y, int width, int height,
+                                                     unsigned image_flags)
 {
     return Gosu_translate_exceptions([=] {
-        std::size_t size = columns * rows * 4;
-        Gosu::Bitmap bitmap{columns, rows};
+        const int pixels = columns * rows * 4;
+        Gosu::Bitmap bitmap;
 
-        if (byte_count == size) {
+        if (byte_count == pixels) {
             // 32 bit per pixel, assume R8G8B8A8
-            std::memcpy(bitmap.data(), blob, size);
+            bitmap = Gosu::Bitmap(columns, rows, Gosu::Buffer(blob, byte_count, nullptr));
         }
-        else if (byte_count == size * sizeof(float)) {
-            // 128 bit per channel, assume float/float/float/float - for Texplay compatibility.
+        else if (byte_count == pixels * 4UL * sizeof(float)) {
+            bitmap.resize(columns, rows);
+            // 128 bit per channel, assume float/float/float/float RGBA - for Texplay compatibility.
             const float* in = static_cast<const float*>(blob);
             Gosu::Color::Channel* out = reinterpret_cast<Gosu::Color::Channel*>(bitmap.data());
-            for (std::size_t i = 0; i < size; ++i) {
+            for (std::size_t i = 0; i < pixels; ++i) {
                 out[i] = static_cast<Gosu::Color::Channel>(in[i] * 255);
             }
         }
         else {
-            throw std::invalid_argument{"Invalid byte_count " + std::to_string(byte_count) +
-                                        "for image of size " + std::to_string(columns) + "x" +
-                                        std::to_string(rows)};
+            throw std::invalid_argument("Invalid byte_count " + std::to_string(byte_count)
+                                        + " for image of size " + std::to_string(columns) + "x"
+                                        + std::to_string(rows));
         }
 
-        return new Gosu_Image{Gosu::Image{bitmap, image_flags}};
+        return new Gosu_Image { Gosu::Image(bitmap, { x, y, width, height }, image_flags) };
     });
 }
 

ffi/Gosu_Image.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "Gosu_FFI.h"
+#include <stddef.h> // for size_t
 #include <stdint.h>
 
 typedef struct Gosu_Image Gosu_Image;
@@ -13,8 +14,13 @@ typedef struct Gosu_GLTexInfo
 
 // Constructor
 GOSU_FFI_API Gosu_Image* Gosu_Image_create(const char* filename, unsigned image_flags);
-GOSU_FFI_API Gosu_Image* Gosu_Image_create_from_blob(void* blob, int byte_count, int width,
-                                                     int height, unsigned image_flags);
+GOSU_FFI_API Gosu_Image* Gosu_Image_create_from_rect(const char* filename, //
+                                                int x, int y, int width, int height,
+                                                unsigned image_flags);
+GOSU_FFI_API Gosu_Image* Gosu_Image_create_from_blob(void* blob, size_t byte_count, //
+                                                     int columns, int rows, //
+                                                     int x, int y, int width, int height,
+                                                     unsigned image_flags);
 GOSU_FFI_API Gosu_Image* Gosu_Image_create_from_markup(const char* markup, const char* font,
                                                        double font_height, int width,
                                                        double spacing, unsigned align,

include/Gosu/Image.hpp

@@ -24,7 +24,7 @@ namespace Gosu
         /// For more flexibility, use the corresponding constructor that uses a Bitmap object.
         explicit Image(const std::string& filename, unsigned image_flags = IF_SMOOTH);
 
-        //! Loads a portion of the the image at the given filename..
+        /// Loads a portion of the the image at the given filename..
         ///
         /// A color key of #ff00ff is automatically applied to BMP image files.
         /// For more flexibility, use the corresponding constructor that uses a Bitmap object.

lib/gosu/ffi.rb

@@ -134,11 +134,12 @@ module GosuFFI
   attach_function :Gosu_Font_set_image, [:pointer, :string, :uint32, :pointer], :void
 
   attach_function :Gosu_Image_create, [:string, :uint32], :pointer
+  attach_function :Gosu_Image_create_from_rect, [:string, :int, :int, :int, :int, :uint32], :pointer
   attach_function :Gosu_Image_destroy, [:pointer], :void
 
   attach_function :Gosu_Image_create_from_markup, [:string, :string, :double, :int, :double, :uint32, :uint32, :uint32], :pointer
   attach_function :Gosu_Image_create_from_text, [:string, :string, :double, :int, :double, :uint32, :uint32, :uint32], :pointer
-  attach_function :Gosu_Image_create_from_blob, [:pointer, :ulong, :int, :int, :uint32], :pointer
+  attach_function :Gosu_Image_create_from_blob, [:pointer, :size_t, :int, :int, :int, :int, :int, :int, :uint32], :pointer
   attach_function :Gosu_Image_create_from_subimage, [:pointer, :int, :int, :int, :int], :pointer
   attach_function :Gosu_Image_create_from_tiles, [:string, :int, :int, :_callback_with_image, :pointer, :uint32], :void
   attach_function :Gosu_Image_create_tiles_from_image, [:pointer, :int, :int, :_callback_with_image, :pointer, :uint32], :void

lib/gosu/image.rb

@@ -1,9 +1,8 @@
 module Gosu
   class Image
-    BlobHelper = Struct.new(:columns, :rows, :to_blob)
-
     def self.from_blob(width, height, rgba = "\0\0\0\0" * (width * height), retro: false, tileable: false)
-      self.new(BlobHelper.new(width, height, rgba), retro: retro, tileable: tileable)
+      @struct ||= Struct.new(:columns, :rows, :to_blob)
+      self.new(@struct.new(width, height, rgba), retro: retro, tileable: tileable)
     end
 
     def self.from_text(markup, line_height, font: Gosu.default_font_name, width: -1, spacing: 0, align: :left,
@@ -48,25 +47,38 @@ def self.load_tiles(filename_or_image, tile_width, tile_height, retro: false, ti
       return images
     end
 
-    def initialize(object, retro: false, tileable: false)
+    def initialize(object, retro: false, tileable: false, rect: nil)
+      if rect and rect.size != 4
+        raise ArgumentError, "Expected 4-element array as rect"
+      end
+
+      flags = GosuFFI.image_flags(retro: retro, tileable: tileable)
+
       if object.is_a? String
-        __image = GosuFFI.Gosu_Image_create(object, GosuFFI.image_flags(retro: retro, tileable: tileable))
+        if rect
+          __image = GosuFFI.Gosu_Image_create_from_rect(object, *rect, flags)
+        else
+          __image = GosuFFI.Gosu_Image_create(object, flags)
+        end
         GosuFFI.check_last_error
       elsif object.is_a?(FFI::Pointer)
         __image = object
-      elsif object.respond_to?(:to_blob) &&
-            object.respond_to?(:columns) &&
+      elsif object.respond_to?(:to_blob) and
+            object.respond_to?(:columns) and
             object.respond_to?(:rows)
-        blob_bytes = object.to_blob { self.format = "RGBA"; self.depth = 8 }.bytes
-        FFI::MemoryPointer.new(:uchar, blob_bytes.size) do |blob|
-          blob.write_array_of_type(:uchar, :put_uchar, blob_bytes)
-          __image = GosuFFI.Gosu_Image_create_from_blob(blob, blob_bytes.size, object.columns, object.rows, GosuFFI.image_flags(retro: retro, tileable: tileable))
-          GosuFFI.check_last_error
-        end
+
+        blob = object.to_blob { self.format = "RGBA"; self.depth = 8 }
 
-        raise "Failed to load image from blob" if __image.null?
+        # This creates a copy of the Ruby string data, which shouldn't be necessary with CRuby.
+        ptr = FFI::MemoryPointer.from_string(blob)
+        rect ||= [0, 0, object.columns, object.rows]
+        # Do not consider the terminating null byte part of the ptr length.
+        __image = GosuFFI.Gosu_Image_create_from_blob(ptr, ptr.size - 1, object.columns, object.rows, *rect, flags)
+        ptr.free
+
+        GosuFFI.check_last_error
       else
-        raise ArgumentError
+        raise ArgumentError, "Expected String or RMagick::Image (or a type compatible with it)"
       end
 
       @managed_pointer = FFI::AutoPointer.new(__image, GosuFFI.method(:Gosu_Image_destroy))
@@ -112,17 +124,17 @@ def to_blob
     end
 
     def subimage(left, top, width, height)
-      __pointer = GosuFFI.Gosu_Image_create_from_subimage(__pointer, left, top, width, height)
+      __subimage = GosuFFI.Gosu_Image_create_from_subimage(__pointer, left, top, width, height)
       GosuFFI.check_last_error
-      Gosu::Image.new(__pointer)
+      Gosu::Image.new(__subimage)
     end
 
     def insert(image, x, y)
       image_ = nil
       if image.is_a?(Gosu::Image)
         image_ = image.__pointer
-      elsif image.respond_to?(:to_blob) &&
-            image.respond_to?(:rows) &&
+      elsif image.respond_to?(:to_blob) and
+            image.respond_to?(:rows) and
             image.respond_to?(:columns)
         image_ = Gosu::Image.new(image).__pointer
       else

test/test_image.rb

@@ -5,13 +5,12 @@ class TestImage < Minitest::Test
 
   private def circle_image(radius)
     # Opaque blue as an RGBA string (0x0000ffff).
-    inside = 0x00.chr + 0x00.chr + 0xff.chr + 0xff.chr
+    inside = "\x00\x00\xff\xff".force_encoding(Encoding::BINARY)
     # Transparent blue as an RGBA string (0x0000ff00).
     outside = 0x00.chr + 0xff.chr + 0xff.chr + 0x00.chr
 
     # This creates a binary string that contains a circle, thanks to the
     # Pythagorean theorem.
-    # Note that the returned size is (radius * 2 + 1), not (radius * 2).
     rgba = (-radius..+radius).map do |y|
       # 'x' is the distance from the center to the edges edge of the circle.
       x = Math.sqrt(radius ** 2 - y ** 2).round
@@ -20,13 +19,36 @@ class TestImage < Minitest::Test
       outside_circle + inside_circle + outside_circle
     end.join
 
+    # The image size is (radius * 2 + 1), not (radius * 2).
     size = radius * 2 + 1
+
     Gosu::Image.from_blob(size, size, rgba)
   end
 
   def test_image_from_blob
     assert_image_matches "test_image/from_blob", circle_image(70), 1.00
   end
+
+  LooksLikeRMagickImage = Struct.new(:columns, :rows, :to_blob)
+
+  def test_image_roundtrips
+    circle = circle_image(70)
+
+    padded_circle = Gosu::Image.from_blob(circle.width + 4, circle.height + 2)
+    padded_circle.insert circle, 2, 1
+
+    # Convert to blob and back.
+    fake_rmagick_image = LooksLikeRMagickImage.new(padded_circle.width, padded_circle.height, padded_circle.to_blob)
+    circle2 = Gosu::Image.new(fake_rmagick_image, rect: [2, 1, circle.width, circle.height])
+    assert circle.similar?(circle2, 1.00)
+
+    # Convert to file and back.
+    temporary_filename = "#{Dir.tmpdir}/gosu_test_image_roundtrip.png"
+    padded_circle.save(temporary_filename)
+    circle3 = Gosu::Image.new(temporary_filename, rect: [2, 1, circle.width, circle.height])
+    assert circle.similar?(circle3, 1.00)
+    File.delete temporary_filename
+  end
 
   private def rect_image(w, h)
     red = 0xff.chr + 0x00.chr + 0x00.chr + 0xff.chr
@@ -50,4 +72,20 @@ def test_image_insert
     end
     assert_image_matches "test_image/insert", canvas, 1.00
   end
+
+  def test_subimage
+    filename = File.join(File.dirname(__FILE__), "test_image_io/no-alpha-jpg.jpg")
+    image = Gosu::Image.new(filename)
+
+    # Passing an out-of-bound rectangle is not allowed.
+    assert_raises(Exception) { image.subimage(0, 0, 100, 1000) }
+
+    # Passing a rectangle that is entirely within the image is allowed.
+    subimage = image.subimage(1, 2, 99, 100)
+    assert_equal 99, subimage.width
+    assert_equal 100, subimage.height
+
+    image_from_rect = Gosu::Image.new(filename, rect: [1, 2, 99, 100])
+    assert image_from_rect.similar?(subimage, 1.00)
+  end
 end