Add warning and correct undo information for unsafe remaps in 8-bit images (fix #715)

src/app/ui/color_bar.cpp

@@ -14,6 +14,7 @@
 #include "app/app.h"
 #include "app/app_menus.h"
 #include "app/cmd/remap_colors.h"
+#include "app/cmd/replace_image.h"
 #include "app/cmd/set_palette.h"
 #include "app/cmd/set_transparent_color.h"
 #include "app/color.h"
@@ -40,12 +41,16 @@
 #include "app/util/clipboard.h"
 #include "base/bind.h"
 #include "base/scoped_value.h"
+#include "doc/cel.h"
+#include "doc/cels_range.h"
 #include "doc/image.h"
 #include "doc/palette.h"
+#include "doc/primitives.h"
 #include "doc/remap.h"
 #include "doc/sort_palette.h"
 #include "doc/sprite.h"
 #include "she/surface.h"
+#include "ui/alert.h"
 #include "ui/graphics.h"
 #include "ui/menu.h"
 #include "ui/message.h"
@@ -57,6 +62,7 @@
 
 #include <cstring>
 
+
 namespace app {
 
 enum class PalButton {
@@ -405,6 +411,16 @@ void ColorBar::onRemapButtonClick()
 {
   ASSERT(m_remap);
 
+  // Check the remap
+  if (!m_remap->isFor8bit() &&
+      Alert::show(
+        "Automatic Remap"
+        "<<The remap operation cannot be perfectly done for more than 256 colors."
+        "<<Do you want to continue anyway?"
+        "||&OK||&Cancel") != 1) {
+    return;
+  }
+
   try {
     ContextWriter writer(UIContext::instance(), 500);
     Sprite* sprite = writer.sprite();
@@ -412,7 +428,20 @@ void ColorBar::onRemapButtonClick()
       ASSERT(sprite->pixelFormat() == IMAGE_INDEXED);
 
       Transaction transaction(writer.context(), "Remap Colors", ModifyDocument);
-      transaction.execute(new cmd::RemapColors(sprite, *m_remap));
+      if (m_remap->isFor8bit()) {
+        transaction.execute(new cmd::RemapColors(sprite, *m_remap));
+      }
+      // Special remap saving original images in undo history
+      else {
+        for (Cel* cel : sprite->uniqueCels()) {
+          ImageRef celImage = cel->imageRef();
+          ImageRef newImage(Image::createCopy(celImage.get()));
+          doc::remap_image(newImage.get(), *m_remap);
+
+          transaction.execute(new cmd::ReplaceImage(
+                                sprite, celImage, newImage));
+        }
+      }
 
       color_t oldTransparent = sprite->transparentColor();
       color_t newTransparent = (*m_remap)[oldTransparent];

src/doc/primitives.cpp

@@ -14,6 +14,7 @@
 #include "doc/brush.h"
 #include "doc/image_impl.h"
 #include "doc/palette.h"
+#include "doc/remap.h"
 #include "doc/rgbmap.h"
 
 #include <stdexcept>
@@ -328,4 +329,19 @@ int count_diff_between_images(const Image* i1, const Image* i2)
   return -1;
 }
 
+void remap_image(Image* image, const Remap& remap)
+{
+  ASSERT(image->pixelFormat() == IMAGE_INDEXED);
+  if (image->pixelFormat() != IMAGE_INDEXED)
+    return;
+
+  LockImageBits<IndexedTraits> bits(image);
+  LockImageBits<IndexedTraits>::iterator
+    it = bits.begin(),
+    end = bits.end();
+
+  for (; it != end; ++it)
+    *it = remap[*it];
+}
+
 } // namespace doc

src/doc/primitives.h

@@ -16,6 +16,7 @@ namespace doc {
   class Brush;
   class Image;
   class Palette;
+  class Remap;
 
   color_t get_pixel(const Image* image, int x, int y);
   void put_pixel(Image* image, int x, int y, color_t c);
@@ -39,6 +40,8 @@ namespace doc {
 
   int count_diff_between_images(const Image* i1, const Image* i2);
 
+  void remap_image(Image* image, const Remap& remap);
+
 } // namespace doc
 
 #endif

src/doc/remap.cpp

@@ -78,4 +78,16 @@ Remap Remap::invert() const
   return inv;
 }
 
+bool Remap::isFor8bit() const
+{
+  for (int i=0; i<size(); ++i) {
+    // Moving entries between [0,255] range to or from [256,+inf)
+    // range are invalid for 8-bit images.
+    if ((i <  256 && m_map[i] >= 256) ||
+        (i >= 256 && m_map[i] <  256))
+      return false;
+  }
+  return true;
+}
+
 } // namespace doc

src/doc/remap.h

@@ -44,6 +44,9 @@ namespace doc {
       return sizeof(*this) + sizeof(int)*size();
     }
 
+    // Returns true if the remap can be safely used in 8-bit images.
+    bool isFor8bit() const;
+
   private:
     std::vector<int> m_map;
   };

src/doc/sprite.cpp

@@ -468,14 +468,7 @@ void Sprite::remapImages(frame_t frameFrom, frame_t frameTo, const Remap& remap)
     // Remap this Cel because is inside the specified range
     if (cel->frame() >= frameFrom &&
         cel->frame() <= frameTo) {
-      Image* image = cel->image();
-      LockImageBits<IndexedTraits> bits(image);
-      LockImageBits<IndexedTraits>::iterator
-        it = bits.begin(),
-        end = bits.end();
-
-      for (; it != end; ++it)
-        *it = remap[*it];
+      remap_image(cel->image(), remap);
     }
   }
 }