WIP add points of interest ᴮᴱᵀᴬ crop type

common-lib/src/main/scala/com/gu/mediaservice/model/Crop.scala

@@ -15,7 +15,14 @@ object Crop {
   def getCropId(b: Bounds) = List(b.x, b.y, b.width, b.height).mkString("_")
 
   def createFromCropSource(by: Option[String], timeRequested: Option[DateTime], specification: CropSpec, master: Option[Asset] = None, cropSizings: List[Asset] = Nil): Crop =
-    Crop(Some(getCropId(specification.bounds)), by, timeRequested, specification, master, cropSizings)
+   Crop(
+      id = Some(getCropId(specification.bounds)),
+      author = by,
+      date = timeRequested,
+      specification = specification,
+      master = master,
+      assets = cropSizings
+    )
 
   def createFromCrop(crop: Crop, master: Asset, assets: List[Asset]): Crop =
     Crop(crop.id, crop.author, crop.date, crop.specification, Some(master), assets)
@@ -43,6 +50,7 @@ object Crop {
 sealed trait ExportType { val name: String }
 case object CropExport extends ExportType { val name = "crop" }
 case object FullExport extends ExportType { val name = "full" }
+case object PointsOfInterestExport extends ExportType { val name = "poi" }
 
 object ExportType {
 
@@ -51,6 +59,7 @@ object ExportType {
   def valueOf(name: String): ExportType = name match {
     case "crop" => CropExport
     case "full" => FullExport
+    case "poi" => PointsOfInterestExport
   }
 
   implicit val exportTypeWrites: Writes[ExportType] = Writes[ExportType](t => JsString(t.name))

cropper/app/controllers/CropperController.scala

@@ -167,16 +167,16 @@ class CropperController(auth: Authentication, crops: Crops, store: CropStore, no
       apiImage <- fetchSourceFromApi(exportRequest.uri, onBehalfOfPrincipal)
       _ <- verify(apiImage.valid, InvalidImage)
       // Image should always have dimensions, but we want to safely extract the Option
-      dimensions <- ifDefined(apiImage.source.dimensions, InvalidImage)
-      cropSpec = ExportRequest.toCropSpec(exportRequest, dimensions)
-      _ <- verify(crops.isWithinImage(cropSpec.bounds, dimensions), InvalidCropRequest)
+      originalDimensions <- ifDefined(apiImage.source.dimensions, InvalidImage)
+      cropSpec = ExportRequest.toCropSpec(exportRequest, originalDimensions)
+      _ <- verify(crops.isWithinImage(cropSpec.bounds, originalDimensions), InvalidCropRequest)
       crop = Crop.createFromCropSource(
         by = Some(Authentication.getIdentity(user)),
         timeRequested = Some(new DateTime()),
         specification = cropSpec
       )
-      markersWithCropDetails = logMarker ++ Map("imageId" -> apiImage.id, "cropId" -> Crop.getCropId(cropSpec.bounds))
-      ExportResult(id, masterSizing, sizings) <- crops.makeExport(apiImage, crop)(markersWithCropDetails)
+      markersWithCropDetails = logMarker ++ Map("imageId" -> apiImage.id, "cropId" -> crop.id)
+      ExportResult(id, masterSizing, sizings) <- crops.makeExport(apiImage, crop, originalDimensions)(markersWithCropDetails)
       finalCrop = Crop.createFromCrop(crop, masterSizing, sizings)
     } yield (id, finalCrop)
   }

cropper/app/lib/Crops.scala

@@ -23,9 +23,9 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
   private val cropQuality = 75d
   private val masterCropQuality = 95d
 
-  def outputFilename(source: SourceImage, bounds: Bounds, outputWidth: Int, fileType: MimeType, isMaster: Boolean = false): String = {
+  def outputFilename(source: SourceImage, cropId: String, outputWidth: Int, fileType: MimeType, isMaster: Boolean = false): String = {
     val masterString: String = if (isMaster) "master/" else ""
-    s"${source.id}/${Crop.getCropId(bounds)}/${masterString}$outputWidth${fileType.fileExtension}"
+    s"${source.id}/$cropId/$masterString$outputWidth${fileType.fileExtension}"
   }
 
   def createMasterCrop(
@@ -36,23 +36,23 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
     colourModel: Option[String],
   )(implicit logMarker: LogMarker): Future[MasterCrop] = {
 
-    val source   = crop.specification
+    val cropSpec = crop.specification
     val metadata = apiImage.metadata
     val iccColourSpace = FileMetadataHelper.normalisedIccColourSpace(apiImage.fileMetadata)
 
     logger.info(logMarker, s"creating master crop for ${apiImage.id}")
 
     for {
       strip <- imageOperations.cropImage(
-        sourceFile, apiImage.source.mimeType, source.bounds, masterCropQuality, config.tempDir,
+        sourceFile, apiImage.source.mimeType, cropSpec.bounds, masterCropQuality, config.tempDir,
         iccColourSpace, colourModel, mediaType, isTransformedFromSource = false
       )
       file: File <- imageOperations.appendMetadata(strip, metadata)
-      dimensions  = Dimensions(source.bounds.width, source.bounds.height)
-      filename    = outputFilename(apiImage, source.bounds, dimensions.width, mediaType, isMaster = true)
+      dimensions  = Dimensions(cropSpec.bounds.width, cropSpec.bounds.height)
+      filename    = outputFilename(apiImage, crop.id.get, dimensions.width, mediaType, isMaster = true)
       sizing      = store.storeCropSizing(file, filename, mediaType, crop, dimensions)
-      dirtyAspect = source.bounds.width.toFloat / source.bounds.height
-      aspect      = crop.specification.aspectRatio.flatMap(AspectRatio.clean).getOrElse(dirtyAspect)
+      dirtyAspect = cropSpec.bounds.width.toFloat / cropSpec.bounds.height
+      aspect      = cropSpec.aspectRatio.flatMap(AspectRatio.clean).getOrElse(dirtyAspect)
     }
     yield MasterCrop(sizing, file, dimensions, aspect)
   }
@@ -64,7 +64,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
       for {
         file          <- imageOperations.resizeImage(sourceFile, apiImage.source.mimeType, dimensions, cropQuality, config.tempDir, cropType)
         optimisedFile = imageOperations.optimiseImage(file, cropType)
-        filename      = outputFilename(apiImage, crop.specification.bounds, dimensions.width, cropType)
+        filename      = outputFilename(apiImage, crop.id.get, dimensions.width, cropType)
         sizing        <- store.storeCropSizing(optimisedFile, filename, cropType, crop, dimensions)
         _             <- delete(file)
         _             <- delete(optimisedFile)
@@ -89,21 +89,28 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
     positiveCoords && strictlyPositiveSize && withinBounds
   }
 
-  def makeExport(apiImage: SourceImage, crop: Crop)(implicit logMarker: LogMarker): Future[ExportResult] = {
-    val source    = crop.specification
+  def makeExport(apiImage: SourceImage, _crop: Crop, originalDimensions: Dimensions)(implicit logMarker: LogMarker): Future[ExportResult] = {
+    val crop = _crop.specification.`type` match {
+      case PointsOfInterestExport => _crop.copy(specification = _crop.specification.copy(bounds = Bounds(0, 0, originalDimensions.width, originalDimensions.height)))
+      case _ => _crop
+    }
+    val cropSpec = crop.specification
     val mimeType = apiImage.source.mimeType.getOrElse(throw MissingMimeType)
     val secureUrl = apiImage.source.secureUrl.getOrElse(throw MissingSecureSourceUrl)
     val colourType = apiImage.fileMetadata.colourModelInformation.getOrElse("colorType", "")
     val hasAlpha = apiImage.fileMetadata.colourModelInformation.get("hasAlpha").flatMap(a => Try(a.toBoolean).toOption).getOrElse(true)
     val cropType = Crops.cropType(mimeType, colourType, hasAlpha)
 
-    Stopwatch(s"making crop assets for ${apiImage.id} ${Crop.getCropId(source.bounds)}") {
+    Stopwatch(s"making crop assets for ${apiImage.id} ${crop.id}") {
       for {
         sourceFile <- tempFileFromURL(secureUrl, "cropSource", "", config.tempDir)
         colourModel <- ImageOperations.identifyColourModel(sourceFile, mimeType)
         masterCrop <- createMasterCrop(apiImage, sourceFile, crop, cropType, colourModel)
 
-        outputDims = dimensionsFromConfig(source.bounds, masterCrop.aspectRatio) :+ masterCrop.dimensions
+        outputDims = dimensionsFromConfig(
+          bounds = cropSpec.bounds,
+          aspectRatio = masterCrop.aspectRatio
+        ) :+ masterCrop.dimensions
 
         sizes <- createCrops(masterCrop.file, outputDims, apiImage, crop, cropType)
         masterSize <- masterCrop.sizing

cropper/app/model/ExportRequest.scala

@@ -16,6 +16,8 @@ case class FullExportRequest(uri: String) extends ExportRequest
 
 case class CropRequest(uri: String, bounds: Bounds, aspectRatio: Option[String]) extends ExportRequest
 
+case class PointsOfInterest(uri: String, bounds: Bounds) extends ExportRequest
+
 
 object ExportRequest {
 
@@ -27,12 +29,18 @@ object ExportRequest {
     (__ \ "aspectRatio").readNullable[String](pattern(aspectRatioLike))
   )(CropRequest.apply _)
 
+  private val readPointsOfInterestRequest: Reads[PointsOfInterest] = (
+    (__ \ "source").read[String] ~
+    __.read[Bounds]
+  )(PointsOfInterest.apply _)
+
   private val readFullExportRequest: Reads[FullExportRequest] =
     (__ \ "source").read[String].map(FullExportRequest.apply)
 
   implicit val readExportRequest: Reads[ExportRequest] = Reads[ExportRequest](jsValue =>
     (jsValue \ "type").validate[String] match {
       case JsSuccess("crop", _) => readCropRequest.reads(jsValue)
+      case JsSuccess("poi", _) => readPointsOfInterestRequest.reads(jsValue)
       case JsSuccess("full", _) => readFullExportRequest.reads(jsValue)
       case _ => JsError("invalid type")
     }
@@ -41,6 +49,8 @@ object ExportRequest {
   def boundsFill(dimensions: Dimensions): Bounds = Bounds(0, 0, dimensions.width, dimensions.height)
 
   def toCropSpec(cropRequest: ExportRequest, dimensions: Dimensions): CropSpec = cropRequest match {
+    case PointsOfInterest(uri, bounds) =>
+      CropSpec(uri, bounds, None, PointsOfInterestExport)
     case FullExportRequest(uri)          =>
       CropSpec(
         uri,

cropper/test/lib/CropsTest.scala

@@ -46,27 +46,27 @@ class CropsTest extends AnyFunSpec with Matchers with MockitoSugar {
   private val store = mock[CropStore]
   private val imageOperations: ImageOperations = mock[ImageOperations]
   private val source: SourceImage = SourceImage("test", mock[Asset], valid = true, mock[ImageMetadata], mock[FileMetadata])
-  private val bounds: Bounds = Bounds(10, 20, 30, 40)
+  private val cropId: String = "10_20_30_40"
   private val outputWidth = 1234
 
   it("should should construct a correct address for a master jpg") {
     val outputFilename = new Crops(config, store, imageOperations)
-      .outputFilename(source, bounds, outputWidth, Jpeg, isMaster = true)
+      .outputFilename(source, cropId, outputWidth, Jpeg, isMaster = true)
     outputFilename shouldBe "test/10_20_30_40/master/1234.jpg"
   }
   it("should should construct a correct address for a non-master jpg") {
     val outputFilename = new Crops(config, store, imageOperations)
-      .outputFilename(source, bounds, outputWidth, Jpeg)
+      .outputFilename(source, cropId, outputWidth, Jpeg)
     outputFilename shouldBe "test/10_20_30_40/1234.jpg"
   }
   it("should should construct a correct address for a non-master tiff") {
     val outputFilename = new Crops(config, store, imageOperations)
-      .outputFilename(source, bounds, outputWidth, Tiff)
+      .outputFilename(source, cropId, outputWidth, Tiff)
     outputFilename shouldBe "test/10_20_30_40/1234.tiff"
   }
   it("should should construct a correct address for a non-master png") {
     val outputFilename = new Crops(config, store, imageOperations)
-      .outputFilename(source, bounds, outputWidth, Png)
+      .outputFilename(source, cropId, outputWidth, Png)
     outputFilename shouldBe "test/10_20_30_40/1234.png"
   }
 }

kahuna/public/js/components/gr-display-crops/gr-display-crops.html

@@ -14,7 +14,7 @@
                 <li class="image-info__group--dl gr-display-crops__crop"
                     ng-repeat="crop in ctrl.crops">
                     <div class="gr-display-crops__crop__list image-info__group--dl__key image-info__heading">
-                        <div>{{crop.specification.aspectRatio | asCropType}}</div>
+                        <div>{{crop.specification | asCropType}}</div>
                         <div>{{crop.master.dimensions.width}} x {{crop.master.dimensions.height}}</div>
                     </div>
                     <ul>

kahuna/public/js/crop/controller.js

@@ -3,7 +3,7 @@ import angular from 'angular';
 import '../components/gr-keyboard-shortcut/gr-keyboard-shortcut';
 import {radioList} from '../components/gr-radio-list/gr-radio-list';
 import {cropUtil} from "../util/crop";
-import {cropOptions} from "../util/constants/cropOptions";
+import {cropOptions, pointsOfInterestBeta} from "../util/constants/cropOptions";
 
 const crop = angular.module('kahuna.crop.controller', [
   'gr.keyboardShortcut',
@@ -150,7 +150,7 @@ crop.controller('ImageCropCtrl', [
 
         ctrl.cropping = true;
 
-        mediaCropper.createCrop(ctrl.image, coords, ratioString)
+        mediaCropper.createCrop(ctrl.image, coords, ratioString, ctrl.cropType === pointsOfInterestBeta.key ? 'poi' : 'crop')
         .then(crop => {
            // Global notification of action
            $rootScope.$emit('events:crop-created', {
@@ -186,6 +186,8 @@ crop.controller('ImageCropCtrl', [
           && cropSettings.shouldShowCropGuttersIfApplicable()
           && maybeCropRatioIfStandard === "5:3";
 
+        ctrl.isPointsOfInterestCrop = newCropType === pointsOfInterestBeta.key; /* TODO adjust the height of easel to avoid scrollbar from explainer */
+
         if (isCropTypeDisabled) {
           ctrl.cropType = oldCropType;
         } else {

kahuna/public/js/crop/view.html

@@ -101,10 +101,15 @@
     Please try to use a larger crop or an alternative image.
 </div>
 
-<div class="warning warning--multiline status--info" ng-if="ctrl.shouldShowVerticalWarningGutters">
+<div class="warning status--info" ng-if="ctrl.shouldShowVerticalWarningGutters">
     Although this is a 5:3 crop, it might be used in a 5:4 space, so please ensure
-    there is nothing important in the striped sides as these might be clipped.<br/>
-    You can also suggest alternative crops for the trail image back in Composer.
+    there is nothing important in the striped sides as these might be clipped.
+</div>
+
+
+<div class="warning status--info" ng-if="ctrl.isPointsOfInterestCrop">
+    This is a 'Points of Interest ᴮᴱᵀᴬ` crop. Use the crop rectangle to cover all the vital points of interest, but nothing more.
+    The frontend can use the crop in spaces of different shapes/ratios ensuring the important parts are always visible.
 </div>
 
 <div class="easel" role="main" aria-label="Image cropper"

kahuna/public/js/image/controller.js

@@ -297,7 +297,11 @@ image.controller('ImageCtrl', [
       }
       ctrl.crop = crops.find(crop => crop.id === cropKey);
       ctrl.fullCrop = crops.find(crop => crop.specification.type === 'full');
-      ctrl.crops = crops.filter(crop => crop.specification.type === 'crop');
+      ctrl.crops = [
+        ...crops.filter(crop => crop.specification.type === 'crop'),
+        // for POI, we can't use crops as the S3 crops are full frame and won't match up
+        ...esCrops.filter(crop => crop.specification.type === 'poi')
+      ];
       ctrl.image.allCrops = ctrl.fullCrop ? [ctrl.fullCrop].concat(ctrl.crops) : ctrl.crops;
       //boolean version for use in template
       ctrl.hasFullCrop = angular.isDefined(ctrl.fullCrop);

kahuna/public/js/image/crop.html

@@ -5,7 +5,7 @@
 <div class="image-crop__info"
      ng-class="{'image-crop__info--selected': crop == ctrl.crop}">
   <div class="flex-container">
-    {{:: crop.specification.aspectRatio | asCropType}}
+    {{:: crop.specification | asCropType}}
     <span class="flex-spacer"></span>
     <span ng-if="crop.specification.aspectRatio">({{:: crop.specification.aspectRatio}})</span>
   </div>
@@ -21,6 +21,6 @@
     data-thumbnail="{{:: extremeAssets.smallest | assetFile}}"
     data-embeddable-url="{{:: ctrl.image.data.id | embeddableUrl:crop.id}}"
     data-aspect-ratio="{{:: crop.specification.aspectRatio}}"
-    data-crop-type="{{:: crop.specification.aspectRatio | asCropType}}"
+    data-crop-type="{{:: crop.specification | asCropType}}"
   ></asset-handle>
 </div>

kahuna/public/js/services/api/media-cropper.js

@@ -16,9 +16,9 @@ cropperApi.factory('mediaCropper', ['mediaApi', function(mediaApi) {
         return cropperRoot;
     }
 
-    function createCrop(image, coords, ratio) {
+    function createCrop(image, coords, ratio, type) {
         return getCropperRoot().follow('crop').post({
-            type: 'crop',
+            type,
             source: image.uri,
             x: coords.x,
             y: coords.y,

kahuna/public/js/util/constants/cropOptions.js

@@ -4,5 +4,6 @@ export const portrait = {key: 'portrait', ratio: 4 / 5, ratioString: '4:5'};
 export const video = {key: 'video', ratio: 16 / 9, ratioString: '16:9'};
 export const square = {key: 'square', ratio: 1, ratioString: '1:1'};
 export const freeform = {key: 'freeform', ratio: null};
+export const pointsOfInterestBeta = {key: 'points of interest ᴮᴱᵀᴬ', ratio:null};
 
-export const cropOptions = [landscape, portrait, video, square, freeform];
+export const cropOptions = [landscape, portrait, video, square, freeform, pointsOfInterestBeta];

kahuna/public/js/util/crop.js

@@ -1,6 +1,6 @@
 import angular from 'angular';
 
-import { landscape, portrait, video, square, freeform, cropOptions } from './constants/cropOptions';
+import {landscape, portrait, video, square, freeform, cropOptions, pointsOfInterestBeta} from './constants/cropOptions';
 
 const CROP_TYPE_STORAGE_KEY = 'cropType';
 const CUSTOM_CROP_STORAGE_KEY = 'customCrop';
@@ -123,8 +123,11 @@ cropUtil.factory('cropSettings', ['storage', function(storage) {
 }]);
 
 cropUtil.filter('asCropType', function() {
-  return ratioString => {
-    const cropSpec = cropOptions.find(_ => _.ratioString === ratioString) || freeform;
+  return specification => {
+    if (specification.type === "poi"){
+      return pointsOfInterestBeta.key;
+    }
+    const cropSpec = cropOptions.find(_ => _.ratioString === specification.aspectRatio) || freeform;
     return cropSpec.key;
   };
 });