Add compression level configuration support for PNG compressor (#17)

Summary:
Solution for issue #2

I'll be glad to hear your minds about the right place to put a predefined set of compression level constants for the Android wrapper. Will it be a good choice to define them right in the `Configuration` class?
Pull Request resolved: #17

Reviewed By: cuva

Differential Revision: D13802377

Pulled By: lambdapioneer

fbshipit-source-id: ba00ef989e0e1918bfac6b379b203cdb3aba2023

android/sample/src/main/java/com/facebook/spectrum/sample/model/ConfigurationViewModel.kt

@@ -74,6 +74,13 @@ enum class ConfigurationViewModel {
             EnumParameterEntry("4:4:4", "1 luma information per chroma information", ImageChromaSamplingMode.S444)
     )
 
+    private val pngCompressionLevel: Array<EnumParameterEntry<Int>> = arrayOf(
+            EnumParameterEntry("Default", null, -1),
+            EnumParameterEntry("Compression level = 0", "No compression", 0),
+            EnumParameterEntry("Compression level = 1", "Best speed", 1),
+            EnumParameterEntry("Compression level = 9", "Best compression", 9)
+    )
+
     private val webpMethodEntries: Array<EnumParameterEntry<Int>> = arrayOf(
             EnumParameterEntry("Default", null, null as Int?),
             EnumParameterEntry("Webp method = 0", "Slowest", 0),
@@ -103,7 +110,8 @@ enum class ConfigurationViewModel {
                     BooleanParameter("PSNR optimised quantisation table", false) { cb, v -> cb.setUsePsnrQuantTable(v) })
             ),
             ParameterGroup("Png", arrayOf(
-                    BooleanParameter("Save with interlacing", false) { cb, v -> cb.setUseInterlacing(v) })
+                    BooleanParameter("Save with interlacing", false) { cb, v -> cb.setUseInterlacing(v) },
+                    EnumParameter("Compression level", pngCompressionLevel) { cb, v -> cb.setCompressionLevel(v) })
             ),
             ParameterGroup("WebP", arrayOf(
                     EnumParameter("Webp compression method", webpMethodEntries) { cb, v -> v?.let { cb.setWebpMethod(it) } },

android/src/main/java/com/facebook/spectrum/Configuration.java

@@ -10,6 +10,7 @@
 import com.facebook.jni.annotations.DoNotStrip;
 import com.facebook.spectrum.image.ImageChromaSamplingMode;
 import com.facebook.spectrum.image.ImageColor;
+import com.facebook.spectrum.utils.Preconditions;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
@@ -71,6 +72,13 @@ public class Configuration {
   /** Png: Whether to save PNG images with interlaced encoding. */
   @DoNotStrip @Nullable public final Boolean useInterlacing;
 
+  /**
+   * Png: The compression level that is used by zlib to determine how much time to spend on trying
+   * to compress the image data. 0 is for not using compression at all and 9 is for the best
+   * compression.
+   */
+  @DoNotStrip @Nullable public final Integer compressionLevel;
+
   /**
    * Webp: Compression quality/speed tradeoff where 0 is the fastest and 6 is the slowest/best
    * compression.
@@ -93,6 +101,7 @@ private Configuration(
       @Nullable Boolean useCompatibleDcScanOpt,
       @Nullable Boolean usePsnrQuantTable,
       @Nullable Boolean useInterlacing,
+      @Nullable Integer compressionLevel,
       @Nullable Integer webpMethod,
       @Nullable ImageHint webpImageHint) {
     this.defaultBackgroundColor = defaultBackgroundColor;
@@ -106,6 +115,7 @@ private Configuration(
     this.chromaSamplingModeOverride = chromaSamplingModeOverride;
     this.usePsnrQuantTable = usePsnrQuantTable;
     this.useInterlacing = useInterlacing;
+    this.compressionLevel = compressionLevel;
     this.webpMethod = webpMethod;
     this.webpImageHint = webpImageHint;
   }
@@ -139,6 +149,8 @@ public String toString() {
         + usePsnrQuantTable
         + ", useInterlacing="
         + useInterlacing
+        + ", compressionLevel="
+        + compressionLevel
         + ", webpMethod="
         + webpMethod
         + ", webpImageHint="
@@ -172,6 +184,7 @@ public static class Builder {
     @Nullable private Boolean mUseCompatibleDcScanOpt;
     @Nullable private Boolean mUsePsnrQuantTable;
     @Nullable private Boolean mUseInterlacing;
+    @Nullable private Integer mCompressionLevel;
     @Nullable private Integer mWebpMethod;
     @Nullable private ImageHint mWebpImageHint;
 
@@ -257,6 +270,17 @@ public Builder setUseInterlacing(final Boolean useInterlacing) {
       return this;
     }
 
+    /**
+     * Png: The compression level that is used by zlib to determine how much time to spend on trying
+     * to compress the image data. 0 is for not using compression at all and 9 is for the best
+     * compression.
+     */
+    public Builder setCompressionLevel(final Integer compressionLevel) {
+      Preconditions.checkArgument(compressionLevel >= 0 && compressionLevel <= 9);
+      mCompressionLevel = compressionLevel;
+      return this;
+    }
+
     /**
      * Webp: Compression quality/speed tradeoff where 0 is the fastest and 6 is the slowest/best
      * compression.
@@ -290,6 +314,7 @@ public Configuration build() {
           mUseCompatibleDcScanOpt,
           mUsePsnrQuantTable,
           mUseInterlacing,
+          mCompressionLevel,
           mWebpMethod,
           mWebpImageHint);
     }
@@ -398,6 +423,9 @@ public boolean equals(Object o) {
     if (useInterlacing != null
         ? !useInterlacing.equals(that.useInterlacing)
         : that.useInterlacing != null) return false;
+    if (compressionLevel != null
+        ? !compressionLevel.equals(that.compressionLevel)
+        : that.compressionLevel != null) return false;
     if (webpMethod != null ? !webpMethod.equals(that.webpMethod) : that.webpMethod != null)
       return false;
     return webpImageHint == that.webpImageHint;

android/src/test/java/com/facebook/spectrum/ConfigurationTest.java

@@ -33,6 +33,7 @@ public void testConfiguration_whenConfigurationDefault_thenAllNull() {
     assertThat(configuration.chromaSamplingModeOverride).isNull();
     assertThat(configuration.usePsnrQuantTable).isNull();
     assertThat(configuration.useInterlacing).isNull();
+    assertThat(configuration.compressionLevel).isNull();
     assertThat(configuration.defaultBackgroundColor).isNull();
     assertThat(configuration.webpMethod).isNull();
     assertThat(configuration.webpImageHint).isNull();
@@ -53,6 +54,7 @@ public void testConfiguration_whenConfigurationSpecified_thenStored() {
             .setChromaSamplingModeOverride(ImageChromaSamplingMode.S444)
             .setUsePsnrQuantTable(true)
             .setUseInterlacing(true)
+            .setCompressionLevel(9)
             .setWebpMethod(1)
             .setWebpImageHint(Configuration.ImageHint.DEFAULT)
             .build();
@@ -68,6 +70,7 @@ public void testConfiguration_whenConfigurationSpecified_thenStored() {
     assertThat(configuration.chromaSamplingModeOverride).isEqualTo(ImageChromaSamplingMode.S444);
     assertThat(configuration.usePsnrQuantTable).isTrue();
     assertThat(configuration.useInterlacing).isTrue();
+    assertThat(configuration.compressionLevel).isEqualTo(9);
     assertThat(configuration.defaultBackgroundColor).isNotNull();
     assertThat(configuration.defaultBackgroundColor.red).isEqualTo(12);
     assertThat(configuration.defaultBackgroundColor.green).isEqualTo(34);

cpp/spectrum/Configuration.cpp

@@ -173,12 +173,23 @@ bool Configuration::Jpeg::operator==(const Jpeg& rhs) const {
 // Png
 //
 
+const Configuration::Png::CompressionLevel
+    Configuration::Png::CompressionLevelNone = 0;
+const Configuration::Png::CompressionLevel
+    Configuration::Png::CompressionLevelBestSpeed = 1;
+const Configuration::Png::CompressionLevel
+    Configuration::Png::CompressionLevelBestCompression = 9;
+const Configuration::Png::CompressionLevel
+    Configuration::Png::CompressionLevelDefault = -1;
+
 void Configuration::Png::merge(const Png& rhs) {
   SPECTRUM_CONFIGURATION_MERGE_PROPERTY(useInterlacing, rhs);
+  SPECTRUM_CONFIGURATION_MERGE_PROPERTY(compressionLevel, rhs);
 }
 
 bool Configuration::Png::operator==(const Png& rhs) const {
-  return SPECTRUM_CONFIGURATION_COMPARE_PROPERTY(useInterlacing, rhs);
+  return SPECTRUM_CONFIGURATION_COMPARE_PROPERTY(useInterlacing, rhs) &&
+      SPECTRUM_CONFIGURATION_COMPARE_PROPERTY(compressionLevel, rhs);
 }
 
 //

cpp/spectrum/Configuration.h

@@ -195,6 +195,12 @@ struct Configuration {
   // LibPng parameters
   //
   struct Png {
+    using CompressionLevel = int;
+    static const CompressionLevel CompressionLevelNone;
+    static const CompressionLevel CompressionLevelBestSpeed;
+    static const CompressionLevel CompressionLevelBestCompression;
+    static const CompressionLevel CompressionLevelDefault;
+
     /**
      * Whether to save PNG images with interlaced encoding.
      */
@@ -203,6 +209,16 @@ struct Configuration {
         useInterlacing,
         false);
 
+    /**
+     * The compression level that is used by zlib to determine
+     * how much time to spend on trying to compress the image data.
+     * 0 is for not using compression at all and 9 is for the best compression.
+     */
+    SPECTRUM_CONFIGURATION_MAKE_PROPERTY_W_DEFAULTS(
+        CompressionLevel,
+        compressionLevel,
+        -1);
+
     void merge(const Png& rhs);
     bool operator==(const Png& rhs) const;
   } png;

cpp/spectrum/plugins/png/LibPngCompressor.cpp

@@ -108,6 +108,9 @@ LibPngCompressor::LibPngCompressor(const codecs::CompressorOptions& options)
         codecs::error::CompressorFailure, "png_create_info_struct_failed");
   }
 
+  png_set_compression_level(
+      libPngWriteStruct, options.configuration.png.compressionLevel());
+
   png_set_write_fn(
       libPngWriteStruct,
       &options.sink,

cpp/test/unit/ConfigurationTest.cpp

@@ -74,6 +74,9 @@ TEST(Configuration, whenRequestingDefaultValue_allParametersAreCorrect) {
 
   // Png
   ASSERT_EQ(false, configuration.png.useInterlacing());
+  ASSERT_EQ(
+      Configuration::Png::CompressionLevelDefault,
+      configuration.png.compressionLevel());
 
   // WebP
   ASSERT_EQ(3, configuration.webp.method());
@@ -150,6 +153,15 @@ TEST(
   SPECTRUM_CONFIGURATION_TEST_PROPERTY(bool, png.useInterlacing, true);
 }
 
+TEST(
+    Configuration_Png,
+    whenMergingOrComparing_thenCompressionLevelIsAccountedFor) {
+  SPECTRUM_CONFIGURATION_TEST_PROPERTY(
+      Configuration::Png::CompressionLevel,
+      png.compressionLevel,
+      Configuration::Png::CompressionLevelBestCompression);
+}
+
 TEST(Configuration_WebP, whenMergingOrComparing_thenMethodAccountedFor) {
   SPECTRUM_CONFIGURATION_TEST_PROPERTY(int, webp.method, 6);
 }

ios/SpectrumKit/SpectrumKit/Configuration/FSPConfigurationPng.h

@@ -8,12 +8,21 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+typedef NSInteger FSPPngCompressionLevel NS_SWIFT_NAME(PngCompressionLevel);
+
+extern const FSPPngCompressionLevel FSPPngCompressionLevelNone NS_SWIFT_NAME(PngCompressionLevelNone);
+extern const FSPPngCompressionLevel FSPPngCompressionLevelBestSpeed NS_SWIFT_NAME(PngCompressionLevelBestSpeed);
+extern const FSPPngCompressionLevel FSPPngCompressionLevelBestCompression NS_SWIFT_NAME(PngCompressionLevelBestCompression);
+extern const FSPPngCompressionLevel FSPPngCompressionLevelDefault NS_SWIFT_NAME(PngCompressionLevelDefault);
+
 NS_SWIFT_NAME(ConfigurationPng)
 @interface FSPConfigurationPng : NSObject <NSCopying>
 
 @property (nonatomic, assign) BOOL useInterlacing;
+@property (nonatomic, assign) FSPPngCompressionLevel compressionLevel;
 
-- (instancetype)initWithUseInterlacing:(BOOL)useInterlacing;
+- (instancetype)initWithUseInterlacing:(BOOL)useInterlacing
+                      compressionLevel:(FSPPngCompressionLevel)compressionLevel;
 
 - (BOOL)isEqualToConfigurationPng:(FSPConfigurationPng *)object;
 

ios/SpectrumKit/SpectrumKit/Configuration/FSPConfigurationPng.mm

@@ -9,8 +9,18 @@
 
 #import <spectrum/Configuration.h>
 
+#import "FSPLog.h"
+
 using namespace facebook::spectrum;
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wglobal-constructors"
+const NSInteger FSPPngCompressionLevelNone = Configuration::Png::CompressionLevelNone;
+const NSInteger FSPPngCompressionLevelBestSpeed = Configuration::Png::CompressionLevelBestSpeed;
+const NSInteger FSPPngCompressionLevelBestCompression = Configuration::Png::CompressionLevelBestCompression;
+const NSInteger FSPPngCompressionLevelDefault = Configuration::Png::CompressionLevelDefault;
+#pragma clang diagnostic pop
+
 @interface FSPConfigurationPng()
 
 @property (nonatomic, assign) Configuration::Png configuration;
@@ -20,9 +30,14 @@ @interface FSPConfigurationPng()
 @implementation FSPConfigurationPng
 
 - (instancetype)initWithUseInterlacing:(BOOL)useInterlacing
+                      compressionLevel:(FSPPngCompressionLevel)compressionLevel
 {
+  FSPReportMustFixIf(compressionLevel < FSPPngCompressionLevelDefault, nil);
+  FSPReportMustFixIf(compressionLevel > FSPPngCompressionLevelBestCompression, nil);
+
   if (self = [super init]) {
     self.useInterlacing = useInterlacing;
+    self.compressionLevel = compressionLevel;
   }
 
   return self;
@@ -40,6 +55,19 @@ - (BOOL)useInterlacing
   return _configuration.useInterlacing();
 }
 
+- (void)setCompressionLevel:(FSPPngCompressionLevel)compressionLevel
+{
+  FSPReportMustFixIf(compressionLevel < FSPPngCompressionLevelDefault, nil);
+  FSPReportMustFixIf(compressionLevel > FSPPngCompressionLevelBestCompression, nil);
+
+  _configuration.compressionLevel(SPECTRUM_CONVERT_OR_THROW(compressionLevel, Configuration::Png::CompressionLevel));
+}
+
+- (FSPPngCompressionLevel)compressionLevel
+{
+  return _configuration.compressionLevel();
+}
+
 #pragma mark - Equality
 
 - (BOOL)isEqualToConfigurationPng:(FSPConfigurationPng *)object
@@ -48,7 +76,7 @@ - (BOOL)isEqualToConfigurationPng:(FSPConfigurationPng *)object
     return NO;
   }
 
-  return self.useInterlacing == object.useInterlacing;
+  return self.useInterlacing == object.useInterlacing && self.compressionLevel == object.compressionLevel;
 }
 
 #pragma mark - NSObject
@@ -75,7 +103,8 @@ - (NSUInteger)hash
 
 - (id)copyWithZone:(__unused NSZone *)zone
 {
-  return [[[self class] allocWithZone:zone] initWithUseInterlacing:self.useInterlacing];
+  return [[[self class] allocWithZone:zone] initWithUseInterlacing:self.useInterlacing
+                                                  compressionLevel:self.compressionLevel];
 }
 
 #pragma mark - Internal

ios/SpectrumKit/SpectrumKitTests/FSPConfigurationTests.mm

@@ -31,6 +31,7 @@ - (void)testDefaultValues
   XCTAssertEqual(configuration.jpeg.usePSNRQuantTable, NO);
 
   XCTAssertEqual(configuration.png.useInterlacing, NO);
+  XCTAssertEqual(configuration.png.compressionLevel, FSPPngCompressionLevelDefault);
 
   XCTAssertEqual(configuration.webp.method, 3);
   XCTAssertEqual(configuration.webp.imageHint, FSPConfigurationWebpImageHintDefault);
@@ -160,6 +161,16 @@ - (void)testIsNotEqualOnDifferentPngUseInterlacing
   XCTAssertNotEqualObjects(object, object2);
 }
 
+- (void)testIsNotEqualOnDifferentPngCompressionLevel
+{
+  const auto object = makeDefaultConfiguration();
+  const auto object2 = makeDefaultConfiguration();
+
+  object2.png.compressionLevel = FSPPngCompressionLevelBestSpeed;
+
+  XCTAssertNotEqualObjects(object, object2);
+}
+
 - (void)testIsNotEqualOnDifferentWebpMethod
 {
   const auto object = makeDefaultConfiguration();
@@ -196,6 +207,7 @@ static Configuration makeDefaultInternalConfiguration()
   configuration.jpeg.useCompatibleDcScanOpt(true);
   configuration.jpeg.usePsnrQuantTable(true);
   configuration.png.useInterlacing(true);
+  configuration.png.compressionLevel(Configuration::Png::CompressionLevelBestCompression);
   configuration.webp.method(2);
   configuration.webp.imageHint(Configuration::Webp::ImageHint::Photo);
   return configuration;
@@ -212,7 +224,8 @@ static Configuration makeDefaultInternalConfiguration()
                                                                   useOptimizeScan:NO
                                                         useCompatibleDCScanOption:YES
                                                                 usePSNRQuantTable:YES];
-  const auto pngConfiguration = [[FSPConfigurationPng alloc] initWithUseInterlacing:YES];
+  const auto pngConfiguration = [[FSPConfigurationPng alloc] initWithUseInterlacing:YES
+                                                                   compressionLevel:FSPPngCompressionLevelBestCompression];
   const auto webpConfiguration = [[FSPConfigurationWebp alloc] initWithMethod:2
                                                                     imageHint:FSPConfigurationWebpImageHintPhoto];
   return [[FSPConfiguration alloc] initWithGeneral:generalConfiguration

ios/SpectrumKitSample/SpectrumKitSample-iOS/Alerts.swift

@@ -162,6 +162,10 @@ struct Alerts {
   static let configurationGeneralDefaultBackgroundColor = AlertContent(title: NSLocalizedString("Select default background color", comment: "Configuration general background color title"),
                                                                        message: nil,
                                                                        optionsType: ConfigurationViewModel.DefaultBackgroundColor.self)
+
+  static let configurationPngCompressionLevel = AlertContent(title: NSLocalizedString("Select PNG compression level", comment: "Configuration PNG compression level"),
+                                                             message: nil,
+                                                             optionsType: ConfigurationViewModel.CompressionLevel.self)
 
   static let configurationWebpMethod = AlertContent(title: NSLocalizedString("Select WebP method", comment: "Configuration webp method title"),
                                                     message: nil,