Allow for cropping transparent true color pngs

common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala

@@ -37,9 +37,16 @@ class S3(credentials: AWSCredentials) {
   }
 
   private def getContentDispositionFilename(image: Image, charset: CharSet): String = {
+
+    val extension = image.source.mimeType match {
+      case Some("image/jpeg") => "jpg"
+      case Some("image/png")  => "png"
+      case _ => throw new Exception("Unsupported mime type")
+    }
+
     val baseFilename: String = image.uploadInfo.filename match {
-      case Some(f)  => s"${removeExtension(f)} (${image.id}).jpg"
-      case _        => s"${image.id}.jpg"
+      case Some(f)  => s"${removeExtension(f)} (${image.id}).${extension}"
+      case _        => s"${image.id}.${extension}"
     }
 
     charset match {

common-lib/src/main/scala/com/gu/mediaservice/lib/imaging/ImageOperations.scala

@@ -10,6 +10,7 @@ import com.gu.mediaservice.model.{Asset, Bounds, Dimensions, ImageMetadata}
 import play.api.libs.concurrent.Execution.Implicits._
 
 import scala.concurrent.Future
+import scala.sys.process._
 
 
 case class ExportResult(id: String, masterCrop: Asset, othersizings: List[Asset])
@@ -48,7 +49,7 @@ object ImageOperations {
         val source = addImage(sourceFile)
 
         val formatter = format(source)("%[JPEG-Colorspace-Name]")
-        
+
         for {
           output <- runIdentifyCmd(formatter)
           colourModel = output.headOption
@@ -78,18 +79,19 @@ object ImageOperations {
   }
 
   def cropImage(sourceFile: File, bounds: Bounds, qual: Double = 100d, tempDir: File,
-                iccColourSpace: Option[String], colourModel: Option[String]): Future[File] = {
+                iccColourSpace: Option[String], colourModel: Option[String], fileType: String): Future[File] = {
     for {
-      outputFile <- createTempFile(s"crop-", ".jpg", tempDir)
-      cropSource  = addImage(sourceFile)
-      qualified   = quality(cropSource)(qual)
-      corrected   = correctColour(qualified)(iccColourSpace, colourModel)
-      converted   = applyOutputProfile(corrected)
-      stripped    = stripMeta(converted)
-      profiled    = applyOutputProfile(stripped)
-      cropped     = crop(profiled)(bounds)
-      addOutput   = addDestImage(cropped)(outputFile)
-      _          <- runConvertCmd(addOutput)
+      outputFile <- createTempFile(s"crop-", s".${fileType}", tempDir)
+      cropSource    = addImage(sourceFile)
+      qualified     = quality(cropSource)(qual)
+      corrected     = correctColour(qualified)(iccColourSpace, colourModel)
+      converted     = applyOutputProfile(corrected)
+      stripped      = stripMeta(converted)
+      profiled      = applyOutputProfile(stripped)
+      cropped       = crop(profiled)(bounds)
+      depthAdjusted = depth(cropped)(8)
+      addOutput     = addDestImage(depthAdjusted)(outputFile)
+      _             <- runConvertCmd(addOutput)
     }
     yield outputFile
   }
@@ -101,9 +103,9 @@ object ImageOperations {
       ).map(_ => sourceFile)
   }
 
-  def resizeImage(sourceFile: File, dimensions: Dimensions, qual: Double = 100d, tempDir: File): Future[File] = {
+  def resizeImage(sourceFile: File, dimensions: Dimensions, qual: Double = 100d, tempDir: File, fileType: String): Future[File] = {
     for {
-      outputFile  <- createTempFile(s"resize-", ".jpg", tempDir)
+      outputFile  <- createTempFile(s"resize-", s".${fileType}", tempDir)
       resizeSource = addImage(sourceFile)
       qualified    = quality(resizeSource)(qual)
       resized      = scale(qualified)(dimensions)
@@ -113,6 +115,21 @@ object ImageOperations {
     yield outputFile
   }
 
+  def optimiseImage(resizedFile: File, mediaType: MimeType): File =
+
+    mediaType.name match {
+      case "image/png" => {
+        val fileName: String = resizedFile.getAbsolutePath()
+
+        val optimisedImageName: String = fileName.split('.')(0) + "optimised.png"
+        Seq("pngquant",  "--quality", "1-85", fileName, "--output", optimisedImageName).!
+
+        new File(optimisedImageName)
+
+      }
+      case "image/jpeg" => resizedFile
+    }
+
   val thumbUnsharpRadius = 0.5d
   val thumbUnsharpSigma = 0.5d
   val thumbUnsharpAmount = 0.8d
@@ -131,4 +148,20 @@ object ImageOperations {
       _          <- runConvertCmd(addOutput)
     } yield outputFile
   }
+
+  sealed trait MimeType {
+    def name: String
+    def extension: String
+  }
+
+  case object Png extends MimeType {
+    val name  = "image/png"
+    val extension = "png"
+  }
+
+  case object Jpeg extends MimeType {
+    val name = "image/jpeg"
+    val extension = "jpg"
+  }
+
 }

common-lib/src/main/scala/com/gu/mediaservice/lib/imaging/im4jwrapper/ImageMagick.scala

@@ -30,6 +30,7 @@ object ImageMagick {
   def resize(op: IMOperation)(maxSize: Int): IMOperation = op <| (_.resize(maxSize, maxSize))
   def scale(op: IMOperation)(dimensions: Dimensions): IMOperation = op <| (_.scale(dimensions.width, dimensions.height))
   def format(op: IMOperation)(definition: String): IMOperation = op <| (_.format(definition))
+  def depth(op: IMOperation)(depth: Int): IMOperation = op <| (_.depth(depth))
   val useGraphicsMagick = true
 
   def runConvertCmd(op: IMOperation): Future[Unit] = Future((new ConvertCmd(useGraphicsMagick)).run(op))

cropper/README.md

@@ -0,0 +1,11 @@
+# cropper
+
+## Requirements
+
+You need to install
+* pngquant
+
+Pngquant should be available from /usr/bin/
+
+If installing pngquant with brew, create a symlink from /usr/local/bin/pngquant to /urs/bin/pngquant.
+

cropper/app/lib/Crops.scala

@@ -2,13 +2,15 @@ package lib
 
 import java.io.File
 
+import com.gu.mediaservice.lib.imaging.ImageOperations.MimeType
 import com.gu.mediaservice.lib.metadata.FileMetadataHelper
 
 import scala.concurrent.Future
 
 import com.gu.mediaservice.model._
 import com.gu.mediaservice.lib.Files
 import com.gu.mediaservice.lib.imaging.{ImageOperations, ExportResult}
+import scala.sys.process._
 
 case object InvalidImage extends Exception("Invalid image cannot be cropped")
 case object MissingMimeType extends Exception("Missing mimeType from source API")
@@ -21,35 +23,54 @@ object Crops {
   import scala.concurrent.ExecutionContext.Implicits.global
   import Files._
 
-  def outputFilename(source: SourceImage, bounds: Bounds, outputWidth: Int, isMaster: Boolean = false): String = {
-    s"${source.id}/${Crop.getCropId(bounds)}/${if(isMaster) "master/" else ""}$outputWidth.jpg"
+  def outputFilename(source: SourceImage, bounds: Bounds, outputWidth: Int, fileType: String, isMaster: Boolean = false): String = {
+    s"${source.id}/${Crop.getCropId(bounds)}/${if(isMaster) "master/" else ""}$outputWidth.${fileType}"
   }
 
-  def createMasterCrop(apiImage: SourceImage, sourceFile: File, crop: Crop, mediaType: String, colourModel: Option[String]): Future[MasterCrop] = {
+  def createMasterCrop(apiImage: SourceImage, sourceFile: File, crop: Crop, mediaType: MimeType, colourModel: Option[String],
+                      colourType: String): Future[MasterCrop] = {
+
     val source   = crop.specification
     val metadata = apiImage.metadata
     val iccColourSpace = FileMetadataHelper.normalisedIccColourSpace(apiImage.fileMetadata)
 
     for {
-      strip <- ImageOperations.cropImage(sourceFile, source.bounds, 100d, Config.tempDir, iccColourSpace, colourModel)
-      file  <- ImageOperations.appendMetadata(strip, metadata)
+      strip <- ImageOperations.cropImage(sourceFile, source.bounds, 100d, Config.tempDir, iccColourSpace, colourModel, mediaType.extension)
+      file: File <- ImageOperations.appendMetadata(strip, metadata)
+
+
+      //Before apps and frontend can handle PNG24s we need to pngquant PNG24 master crops
+      optimisedFile =  if (colourType == "True Color with Alpha") {
+
+        val fileName = file.getAbsolutePath()
+
+
+        val optimisedImageName: String = fileName.split('.')(0) + "optimised.png"
+        Seq("pngquant", "--quality", "1-85", fileName, "--output", optimisedImageName).!
+        new File(optimisedImageName)
+      } else file
 
       dimensions  = Dimensions(source.bounds.width, source.bounds.height)
-      filename    = outputFilename(apiImage, source.bounds, dimensions.width, true)
-      sizing      = CropStore.storeCropSizing(file, filename, mediaType, crop, dimensions)
+      filename    = outputFilename(apiImage, source.bounds, dimensions.width, mediaType.extension, true)
+      sizing      = CropStore.storeCropSizing(file, filename, mediaType.name, crop, dimensions)
       dirtyAspect = source.bounds.width.toFloat / source.bounds.height
       aspect      = crop.specification.aspectRatio.flatMap(AspectRatio.clean(_)).getOrElse(dirtyAspect)
+
     }
-    yield MasterCrop(sizing, file, dimensions, aspect)
+    yield MasterCrop(sizing, optimisedFile, dimensions, aspect)
   }
 
-  def createCrops(sourceFile: File, dimensionList: List[Dimensions], apiImage: SourceImage, crop: Crop, mediaType: String): Future[List[Asset]] = {
+  def createCrops(sourceFile: File, dimensionList: List[Dimensions], apiImage: SourceImage, crop: Crop,
+                  mediaType: MimeType): Future[List[Asset]] = {
+
     Future.sequence[Asset, List](dimensionList.map { dimensions =>
       for {
-        file       <- ImageOperations.resizeImage(sourceFile, dimensions, 75d, Config.tempDir)
-        filename    = outputFilename(apiImage, crop.specification.bounds, dimensions.width)
-        sizing     <- CropStore.storeCropSizing(file, filename, mediaType, crop, dimensions)
-        _          <- delete(file)
+        file          <- ImageOperations.resizeImage(sourceFile, dimensions, 75d, Config.tempDir, mediaType.extension)
+        optimisedFile = ImageOperations.optimiseImage(file, mediaType)
+        filename      = outputFilename(apiImage, crop.specification.bounds, dimensions.width, mediaType.extension)
+        sizing       <- CropStore.storeCropSizing(optimisedFile, filename, mediaType.extension, crop, dimensions)
+        _            <- delete(file)
+        _            <- delete(optimisedFile)
       }
       yield sizing
     })
@@ -75,12 +96,17 @@ object Crops {
     val source    = crop.specification
     val mediaType = apiImage.source.mimeType.getOrElse(throw MissingMimeType)
     val secureUrl = apiImage.source.secureUrl.getOrElse(throw MissingSecureSourceUrl)
-    val cropType = "image/jpeg"
+    val colourType = apiImage.fileMetadata.colourModelInformation.get("colorType").getOrElse("")
+
+    val cropType = if (mediaType == "image/png" && colourType != "True Color")
+      ImageOperations.Png
+    else
+      ImageOperations.Jpeg
 
     for {
       sourceFile  <- tempFileFromURL(secureUrl, "cropSource", "", Config.tempDir)
       colourModel <- ImageOperations.identifyColourModel(sourceFile, mediaType)
-      masterCrop  <- createMasterCrop(apiImage, sourceFile, crop, cropType, colourModel)
+      masterCrop  <- createMasterCrop(apiImage, sourceFile, crop, cropType, colourModel, colourType)
 
       outputDims = dimensionsFromConfig(source.bounds, masterCrop.aspectRatio) :+ masterCrop.dimensions
 
@@ -91,5 +117,4 @@ object Crops {
     }
     yield ExportResult(apiImage.id, masterSize, sizes)
   }
-
 }

kahuna/public/js/components/gr-icon/gr-icon.js

@@ -28,11 +28,12 @@ icon.directive('grIconLabel', [function () {
     return {
         restrict: 'E',
         scope: {
-            grIcon: '@'
+            grIcon: '@',
+            grLoading: '@'
         },
         transclude: 'replace',
         template: `
-            <gr-icon>{{grIcon}}</gr-icon>
+            <gr-icon ng-class="{'spin': grLoading === 'true'}">{{grIcon}}</gr-icon>
             <span class="icon-label"><ng:transclude></ng:transclude></span>`
     };
 }]);

kahuna/public/js/crop/view.html

@@ -100,7 +100,7 @@
                     ng:click="ctrl.callCrop()"
                     ng:disabled="ctrl.cropping"
                     gr-tooltip="Crop [enter]">
-                <gr-icon-label gr-icon="crop">Crop<span ng:show="ctrl.cropping">ping…</span></gr-icon-label>
+                <gr-icon-label gr-icon="crop" gr-loading="{{ctrl.cropping}}">Crop<span ng:show="ctrl.cropping">ping…</span></gr-icon-label>
             </button>
         </div>
     </gr-top-bar-actions>

kahuna/public/js/edits/image-editor.html

@@ -67,11 +67,8 @@
                 <span class="result-editor__status status status--invalid"
                       ng:switch-when="invalid">
                         Invalid
-                    <gr-icon title="You need to add both a credit and description, (PNGs are also invalid).">help</gr-icon>
+                    <gr-icon title="You need to add both a credit and description.">help</gr-icon>
                 </span>
-                <div class="result-editor__info-item--png-warning"ng:if="ctrl.invalidReasons['is_invalid_png']">
-                    <strong>PNG</strong>: Pngs with transparency not supported!
-                </div>
             </div>
 
             <gr-archiver-status class="result-editor__archiver"

kahuna/public/js/services/image/downloads.js

@@ -16,12 +16,13 @@ imageDownloadsService.factory('imageDownloadsService', ['imgops', '$http', funct
     function imageName(imageData) {
         const filename = imageData.uploadInfo.filename;
         const imageId = imageData.id;
+        const extension = imageData.source.mimeType === 'image/jpeg' ? 'jpg' : 'png';
 
         if (filename) {
             const basename = stripExtension(filename);
-            return `${basename} (${imageId}).jpg`;
+            return `${basename} (${imageId}).` + extension;
         } else {
-            return `${imageId}.jpg`;
+            return `${imageId}` + extension;
         }
     }
 

media-api/app/lib/ImageExtras.scala

@@ -10,23 +10,18 @@ object ImageExtras {
 
   val validityDescription = Map(
     "missing_credit"              -> "Missing credit information",
-    "missing_description"         -> "Missing description",
-    "is_invalid_png"              -> "PNG images with transparency cannot be used"
+    "missing_description"         -> "Missing description"
   )
 
   private def optToBool[T](o: Option[T]): Boolean =
     o.map(_ => true).getOrElse(false)
 
   def hasCredit(meta: ImageMetadata) = optToBool(meta.credit)
   def hasDescription(meta: ImageMetadata) = optToBool(meta.description)
-  def isInvalidPng(image: Image) =
-    image.source.mimeType == Some("image/png") &&
-      image.fileMetadata.colourModelInformation.get("colorType").getOrElse("") != "True Color"
 
   def validityMap(image: Image): Map[String, Boolean] = Map(
     "missing_credit"              -> !hasCredit(image.metadata),
-    "missing_description"         -> !hasDescription(image.metadata),
-    "is_invalid_png"              -> isInvalidPng(image)
+    "missing_description"         -> !hasDescription(image.metadata)
   )
 
   def invalidReasons(validityMap: Map[String, Boolean]) = validityMap