Ensure removeAlpha removes all alpha channels #2266

docs/src/content/docs/api-channel.md

@@ -6,7 +6,7 @@ title: Channel manipulation
 ## removeAlpha
 > removeAlpha() ⇒ <code>Sharp</code>
 
-Remove alpha channel, if any. This is a no-op if the image does not have an alpha channel.
+Remove alpha channels, if any. This is a no-op if the image does not have an alpha channel.
 
 See also [flatten](/api-operation#flatten).
 

docs/src/content/docs/changelog.md

@@ -11,6 +11,9 @@ Requires libvips v8.16.0
 * Breaking: Support array of input images to be joined or animated.
   [#1580](https://github.com/lovell/sharp/issues/1580)
 
+* Breaking: Ensure `removeAlpha` removes all alpha channels.
+  [#2266](https://github.com/lovell/sharp/issues/2266)
+
 * Breaking: Support `info.size` on wide-character systems via upgrade to C++17.
   [#3943](https://github.com/lovell/sharp/issues/3943)
 

lib/channel.js

@@ -16,7 +16,7 @@ const bool = {
 };
 
 /**
- * Remove alpha channel, if any. This is a no-op if the image does not have an alpha channel.
+ * Remove alpha channels, if any. This is a no-op if the image does not have an alpha channel.
  *
  * See also {@link /api-operation#flatten|flatten}.
  *

src/common.cc

@@ -595,14 +595,6 @@ namespace sharp {
     return image;
   }
 
-  /*
-    Does this image have an alpha channel?
-    Uses colour space interpretation with number of channels to guess this.
-  */
-  bool HasAlpha(VImage image) {
-    return image.has_alpha();
-  }
-
   static void* RemoveExifCallback(VipsImage *image, char const *field, GValue *value, void *data) {
     std::vector<std::string> *fieldNames = static_cast<std::vector<std::string> *>(data);
     std::string fieldName(field);
@@ -1011,13 +1003,13 @@ namespace sharp {
       };
     }
     // Add alpha channel to alphaColour colour
-    if (colour[3] < 255.0 || HasAlpha(image)) {
+    if (colour[3] < 255.0 || image.has_alpha()) {
       alphaColour.push_back(colour[3] * multiplier);
     }
     // Ensure alphaColour colour uses correct colourspace
     alphaColour = sharp::GetRgbaAsColourspace(alphaColour, image.interpretation(), premultiply);
     // Add non-transparent alpha channel, if required
-    if (colour[3] < 255.0 && !HasAlpha(image)) {
+    if (colour[3] < 255.0 && !image.has_alpha()) {
       image = image.bandjoin(
         VImage::new_matrix(image.width(), image.height()).new_from_image(255 * multiplier).cast(image.format()));
     }
@@ -1025,10 +1017,10 @@ namespace sharp {
   }
 
   /*
-    Removes alpha channel, if any.
+    Removes alpha channels, if any.
   */
   VImage RemoveAlpha(VImage image) {
-    if (HasAlpha(image)) {
+    while (image.bands() > 1 && image.has_alpha()) {
       image = image.extract_band(0, VImage::option()->set("n", image.bands() - 1));
     }
     return image;
@@ -1038,7 +1030,7 @@ namespace sharp {
     Ensures alpha channel, if missing.
   */
   VImage EnsureAlpha(VImage image, double const value) {
-    if (!HasAlpha(image)) {
+    if (!image.has_alpha()) {
       std::vector<double> alpha;
       alpha.push_back(value * sharp::MaximumImageAlpha(image.interpretation()));
       image = image.bandjoin_const(alpha);

src/common.h

@@ -245,12 +245,6 @@ namespace sharp {
   */
   VImage SetProfile(VImage image, std::pair<char*, size_t> icc);
 
-  /*
-    Does this image have an alpha channel?
-    Uses colour space interpretation with number of channels to guess this.
-  */
-  bool HasAlpha(VImage image);
-
   /*
     Remove all EXIF-related image fields.
   */
@@ -381,7 +375,7 @@ namespace sharp {
   std::tuple<VImage, std::vector<double>> ApplyAlpha(VImage image, std::vector<double> colour, bool premultiply);
 
   /*
-    Removes alpha channel, if any.
+    Removes alpha channels, if any.
   */
   VImage RemoveAlpha(VImage image);
 

src/metadata.cc

@@ -95,7 +95,7 @@ class MetadataWorker : public Napi::AsyncWorker {
         baton->background = image.get_array_double("background");
       }
       // Derived attributes
-      baton->hasAlpha = sharp::HasAlpha(image);
+      baton->hasAlpha = image.has_alpha();
       baton->orientation = sharp::ExifOrientation(image);
       // EXIF
       if (image.get_typeof(VIPS_META_EXIF_NAME) == VIPS_TYPE_BLOB) {

src/operations.cc

@@ -40,7 +40,7 @@ namespace sharp {
       typeBeforeTint = VIPS_INTERPRETATION_sRGB;
     }
     // Apply lookup table
-    if (HasAlpha(image)) {
+    if (image.has_alpha()) {
       VImage alpha = image[image.bands() - 1];
       image = RemoveAlpha(image)
         .colourspace(VIPS_INTERPRETATION_B_W)
@@ -83,7 +83,7 @@ namespace sharp {
       // Scale luminance, join to chroma, convert back to original colourspace
       VImage normalized = luminance.linear(f, a).bandjoin(chroma).colourspace(typeBeforeNormalize);
       // Attach original alpha channel, if any
-      if (HasAlpha(image)) {
+      if (image.has_alpha()) {
         // Extract original alpha channel
         VImage alpha = image[image.bands() - 1];
         // Join alpha channel to normalised image
@@ -106,7 +106,7 @@ namespace sharp {
    * Gamma encoding/decoding
    */
   VImage Gamma(VImage image, double const exponent) {
-    if (HasAlpha(image)) {
+    if (image.has_alpha()) {
       // Separate alpha channel
       VImage alpha = image[image.bands() - 1];
       return RemoveAlpha(image).gamma(VImage::option()->set("exponent", exponent)).bandjoin(alpha);
@@ -132,7 +132,7 @@ namespace sharp {
    * Produce the "negative" of the image.
    */
   VImage Negate(VImage image, bool const negateAlpha) {
-    if (HasAlpha(image) && !negateAlpha) {
+    if (image.has_alpha() && !negateAlpha) {
       // Separate alpha channel
       VImage alpha = image[image.bands() - 1];
       return RemoveAlpha(image).invert().bandjoin(alpha);
@@ -205,7 +205,7 @@ namespace sharp {
   VImage Modulate(VImage image, double const brightness, double const saturation,
                   int const hue, double const lightness) {
     VipsInterpretation colourspaceBeforeModulate = image.interpretation();
-    if (HasAlpha(image)) {
+    if (image.has_alpha()) {
       // Separate alpha channel
       VImage alpha = image[image.bands() - 1];
       return RemoveAlpha(image)
@@ -297,7 +297,7 @@ namespace sharp {
       threshold *= 256.0;
     }
     std::vector<double> backgroundAlpha({ background.back() });
-    if (HasAlpha(image)) {
+    if (image.has_alpha()) {
       background.pop_back();
     } else {
       background.resize(image.bands());
@@ -307,7 +307,7 @@ namespace sharp {
       ->set("background", background)
       ->set("line_art", lineArt)
       ->set("threshold", threshold));
-    if (HasAlpha(image)) {
+    if (image.has_alpha()) {
       // Search alpha channel (A)
       int leftA, topA, widthA, heightA;
       VImage alpha = image[image.bands() - 1];
@@ -344,7 +344,7 @@ namespace sharp {
       throw VError("Band expansion using linear is unsupported");
     }
     bool const uchar = !Is16Bit(image.interpretation());
-    if (HasAlpha(image) && a.size() != bands && (a.size() == 1 || a.size() == bands - 1 || bands - 1 == 1)) {
+    if (image.has_alpha() && a.size() != bands && (a.size() == 1 || a.size() == bands - 1 || bands - 1 == 1)) {
       // Separate alpha channel
       VImage alpha = image[bands - 1];
       return RemoveAlpha(image).linear(a, b, VImage::option()->set("uchar", uchar)).bandjoin(alpha);
@@ -357,7 +357,7 @@ namespace sharp {
    * Unflatten
    */
   VImage Unflatten(VImage image) {
-    if (HasAlpha(image)) {
+    if (image.has_alpha()) {
       VImage alpha = image[image.bands() - 1];
       VImage noAlpha = RemoveAlpha(image);
       return noAlpha.bandjoin(alpha & (noAlpha.colourspace(VIPS_INTERPRETATION_B_W) < 255));

src/pipeline.cc

@@ -51,11 +51,11 @@ class PipelineWorker : public Napi::AsyncWorker {
           std::tie(image, inputImageType) = sharp::OpenInput(join);
           image = sharp::EnsureColourspace(image, baton->colourspacePipeline);
           images.push_back(image);
-          hasAlpha |= sharp::HasAlpha(image);
+          hasAlpha |= image.has_alpha();
         }
         if (hasAlpha) {
           for (auto &image : images) {
-            if (!sharp::HasAlpha(image)) {
+            if (!image.has_alpha()) {
               image = sharp::EnsureAlpha(image, 1);
             }
           }
@@ -372,7 +372,7 @@ class PipelineWorker : public Napi::AsyncWorker {
       }
 
       // Flatten image to remove alpha channel
-      if (baton->flatten && sharp::HasAlpha(image)) {
+      if (baton->flatten && image.has_alpha()) {
         image = sharp::Flatten(image, baton->flattenBackground);
       }
 
@@ -392,12 +392,12 @@ class PipelineWorker : public Napi::AsyncWorker {
       bool const shouldSharpen = baton->sharpenSigma != 0.0;
       bool const shouldComposite = !baton->composite.empty();
 
-      if (shouldComposite && !sharp::HasAlpha(image)) {
+      if (shouldComposite && !image.has_alpha()) {
         image = sharp::EnsureAlpha(image, 1);
       }
 
       VipsBandFormat premultiplyFormat = image.format();
-      bool const shouldPremultiplyAlpha = sharp::HasAlpha(image) &&
+      bool const shouldPremultiplyAlpha = image.has_alpha() &&
         (shouldResize || shouldBlur || shouldConv || shouldSharpen);
 
       if (shouldPremultiplyAlpha) {
@@ -725,9 +725,7 @@ class PipelineWorker : public Napi::AsyncWorker {
           }
           // Ensure image to composite is sRGB with unpremultiplied alpha
           compositeImage = compositeImage.colourspace(VIPS_INTERPRETATION_sRGB);
-          if (!sharp::HasAlpha(compositeImage)) {
+          compositeImage = sharp::EnsureAlpha(compositeImage, 1);
-            compositeImage = sharp::EnsureAlpha(compositeImage, 1);
-          }
           if (composite->premultiplied) compositeImage = compositeImage.unpremultiply();
           // Calculate position
           int left;
@@ -828,7 +826,7 @@ class PipelineWorker : public Napi::AsyncWorker {
       // Extract channel
       if (baton->extractChannel > -1) {
         if (baton->extractChannel >= image.bands()) {
-          if (baton->extractChannel == 3 && sharp::HasAlpha(image)) {
+          if (baton->extractChannel == 3 && image.has_alpha()) {
             baton->extractChannel = image.bands() - 1;
           } else {
             (baton->err)
@@ -1052,7 +1050,7 @@ class PipelineWorker : public Napi::AsyncWorker {
         } else if (baton->formatOut == "dz") {
           // Write DZ to buffer
           baton->tileContainer = VIPS_FOREIGN_DZ_CONTAINER_ZIP;
-          if (!sharp::HasAlpha(image)) {
+          if (!image.has_alpha()) {
             baton->tileBackground.pop_back();
           }
           image = sharp::StaySequential(image, baton->tileAngle != 0);
@@ -1256,7 +1254,7 @@ class PipelineWorker : public Napi::AsyncWorker {
           if (isDzZip) {
             baton->tileContainer = VIPS_FOREIGN_DZ_CONTAINER_ZIP;
           }
-          if (!sharp::HasAlpha(image)) {
+          if (!image.has_alpha()) {
             baton->tileBackground.pop_back();
           }
           image = sharp::StaySequential(image, baton->tileAngle != 0);

src/stats.cc

@@ -58,7 +58,7 @@ class StatsWorker : public Napi::AsyncWorker {
           baton->channelStats.push_back(cStats);
         }
         // Image is not opaque when alpha layer is present and contains a non-mamixa value
-        if (sharp::HasAlpha(image)) {
+        if (image.has_alpha()) {
           double const minAlpha = static_cast<double>(stats.getpoint(STAT_MIN_INDEX, bands).front());
           if (minAlpha != sharp::MaximumImageAlpha(image.interpretation())) {
             baton->isOpaque = false;

src/utilities.cc

@@ -233,10 +233,10 @@ Napi::Value _maxColourDistance(const Napi::CallbackInfo& info) {
   double maxColourDistance;
   try {
     // Premultiply and remove alpha
-    if (sharp::HasAlpha(image1)) {
+    if (image1.has_alpha()) {
       image1 = image1.premultiply().extract_band(1, VImage::option()->set("n", image1.bands() - 1));
     }
-    if (sharp::HasAlpha(image2)) {
+    if (image2.has_alpha()) {
       image2 = image2.premultiply().extract_band(1, VImage::option()->set("n", image2.bands() - 1));
     }
     // Calculate colour distance