Merge pull request #1098 from CounterPillow/optimizehints

Clarify optimizeimg docs; Warn if the order of optimizers does not make sense.

docs/config.rst

@@ -557,7 +557,9 @@ values. The valid configuration keys are listed below.
     The option is a list of Optimizer objects, which are then executed in
     the order in which they're specified::
         
+        # Import the optimizers we need
         from optimizeimages import pngnq, optipng
+
         worlds["world"] = "/path/to/world"
 
         renders["daytime"] = {
@@ -567,6 +569,10 @@ values. The valid configuration keys are listed below.
             "optimizeimg":[pngnq(sampling=1), optipng(olevel=3)],
         }
 
+    .. note::
+        Don't forget to import the optimizers you use in your config file, as shown in the
+        example above.
+    
     Here is a list of supported image optimization programs:
 
     ``pngnq``
@@ -612,8 +618,9 @@ values. The valid configuration keys are listed below.
             **Default:** ``2``
 
     ``pngcrush``
-        pngcrush is very slow and not very good, you should use optipng in probably all cases.
+        pngcrush, like optipng, is a lossless PNG recompressor. If you are able to do so, it
-        However, Overviewer still allows you to use it because we're nice people like that.
+        is recommended to use optipng instead, as it generally yields better results in less
+        time.
         Available settings:
 
         ``brute``
@@ -626,10 +633,6 @@ values. The valid configuration keys are listed below.
 
             **Default:** ``False``
 
-    .. note::
-        Don't forget to import the optimizers you use in your settings file, as shown in the
-        example above.
-
     **Default:** ``[]``
 
 ``bgcolor``

overviewer_core/optimizeimages.py

@@ -40,6 +40,10 @@ class Optimizer:
         if (not exists_in_path(self.binaryname)) and (not exists_in_path(self.binaryname + ".exe")):
             raise Exception("Optimization program '%s' was not found!" % self.binaryname)
     
+    def is_crusher(self):
+        """Should return True if the optimization is lossless, i.e. none of the actual image data will be changed."""
+        raise NotImplementedError("I'm so abstract I can't even say whether I'm a crusher.")
+        
 
 class NonAtomicOptimizer(Optimizer):
     def cleanup(self, img):
@@ -84,6 +88,9 @@ class pngnq(NonAtomicOptimizer, PNGOptimizer):
 
         NonAtomicOptimizer.fire_and_forget(self, args, img)
 
+    def is_crusher(self):
+        return False
+
 class pngcrush(NonAtomicOptimizer, PNGOptimizer):
     binaryname = "pngcrush"
     # really can't be bothered to add some interface for all
@@ -98,6 +105,9 @@ class pngcrush(NonAtomicOptimizer, PNGOptimizer):
 
         NonAtomicOptimizer.fire_and_forget(self, args, img)
 
+    def is_crusher(self):
+        return True
+
 class optipng(Optimizer, PNGOptimizer):
     binaryname = "optipng"
 
@@ -107,6 +117,9 @@ class optipng(Optimizer, PNGOptimizer):
     def optimize(self, img):
         Optimizer.fire_and_forget(self, [self.binaryname, "-o" + str(self.olevel), "-quiet", img])
 
+    def is_crusher(self):
+        return True
+        
 
 def optimize_image(imgpath, imgformat, optimizers):
         for opt in optimizers:

overviewer_core/settingsValidators.py

@@ -160,15 +160,25 @@ def validateBGColor(color):
 def validateOptImg(optimizers):
     if isinstance(optimizers, (int, long)):
         from optimizeimages import pngcrush
-        logging.warning("You're using a deprecated definition of optimizeimg. We'll do what you say for now, but please fix this as soon as possible.")
+        logging.warning("You're using a deprecated definition of optimizeimg. "\
+                        "We'll do what you say for now, but please fix this as soon as possible.")
         optimizers = [pngcrush()]
     if not isinstance(optimizers, list):
-        raise ValidationException("optimizeimg is not a list. Make sure you specify them like [foo()], with square brackets.")
+        raise ValidationException("What you passed to optimizeimg is not a list. "\
-    for opt in optimizers:
+                                  "Make sure you specify them like [foo()], with square brackets.")
-        if not isinstance(opt, Optimizer):
-            raise ValidationException("Invalid Optimizer!")
 
-        opt.check_availability()
+    if optimizers:
+        for opt, next_opt in zip(optimizers, optimizers[1:]) + [(optimizers[-1], None)]:
+            if not isinstance(opt, Optimizer):
+                raise ValidationException("Invalid Optimizer!")
+
+            opt.check_availability()
+
+            # Check whether the chaining is somewhat sane
+            if next_opt:
+                if opt.is_crusher() and not next_opt.is_crusher():
+                    logging.warning("You're feeding a crushed output into an optimizer that does not crush. "\
+                                    "This is most likely pointless, and wastes time.")
 
     return optimizers