Skip to content

Commit

Permalink
WIP add points of interest ᴮᴱᵀᴬ crop type
Browse files Browse the repository at this point in the history
Co-authored-by: Frederick O'Brien <[email protected]>
  • Loading branch information
twrichards and frederickobrien committed Nov 1, 2024
1 parent 427322f commit 7795c33
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 22 deletions.
28 changes: 24 additions & 4 deletions common-lib/src/main/scala/com/gu/mediaservice/model/Crop.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,28 @@ case class Crop(id: Option[String], author: Option[String], date: Option[DateTim
object Crop {
import com.gu.mediaservice.lib.formatting._

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)
private def getCropId(b: Bounds): String = List(b.x, b.y, b.width, b.height).mkString("_")
private def getPoiCropId(b: Bounds, fullDimensions: Dimensions): String = s"${getCropId(b)}_${fullDimensions.width}_${fullDimensions.height}"

def createFromCropSource(by: Option[String], timeRequested: Option[DateTime], specification: CropSpec, master: Option[Asset] = None, cropSizings: List[Asset] = Nil, maybeFullDimensions: Option[Dimensions] = None): Crop =
(specification.`type`, maybeFullDimensions) match {
case (PointsOfInterestExport, Some(fullDimensions)) => Crop(
id = Some(getPoiCropId(specification.bounds, fullDimensions)),
author = by,
date = timeRequested,
specification = specification.copy(bounds = Bounds(0, 0, fullDimensions.width, fullDimensions.height)),
master = master,
assets = cropSizings
)
case _ => 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)
Expand Down Expand Up @@ -43,6 +61,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 {

Expand All @@ -51,6 +70,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))
Expand Down
5 changes: 3 additions & 2 deletions cropper/app/controllers/CropperController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,10 @@ class CropperController(auth: Authentication, crops: Crops, store: CropStore, no
crop = Crop.createFromCropSource(
by = Some(Authentication.getIdentity(user)),
timeRequested = Some(new DateTime()),
specification = cropSpec
specification = cropSpec,
maybeFullDimensions = Some(dimensions)
)
markersWithCropDetails = logMarker ++ Map("imageId" -> apiImage.id, "cropId" -> Crop.getCropId(cropSpec.bounds))
markersWithCropDetails = logMarker ++ Map("imageId" -> apiImage.id, "cropId" -> crop.id)
ExportResult(id, masterSizing, sizings) <- crops.export(apiImage, crop)(markersWithCropDetails)
finalCrop = Crop.createFromCrop(crop, masterSizing, sizings)
} yield (id, finalCrop)
Expand Down
10 changes: 5 additions & 5 deletions cropper/app/lib/Crops.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -49,7 +49,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
)
file: File <- imageOperations.appendMetadata(strip, metadata)
dimensions = Dimensions(source.bounds.width, source.bounds.height)
filename = outputFilename(apiImage, source.bounds, dimensions.width, mediaType, isMaster = true)
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)
Expand All @@ -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)
Expand Down Expand Up @@ -97,7 +97,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
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)
Expand Down
10 changes: 10 additions & 0 deletions cropper/app/model/ExportRequest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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")
}
Expand All @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions cropper/test/lib/CropsTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
6 changes: 4 additions & 2 deletions kahuna/public/js/crop/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
import {getFeatureSwitchActive} from "../components/gr-feature-switch-panel/gr-feature-switch-panel";

const crop = angular.module('kahuna.crop.controller', [
Expand Down Expand Up @@ -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', {
Expand Down Expand Up @@ -187,6 +187,8 @@ crop.controller('ImageCropCtrl', [
&& getFeatureSwitchActive("show-cropping-gutters-switch")
&& 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 {
Expand Down
6 changes: 6 additions & 0 deletions kahuna/public/js/crop/view.html
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,12 @@
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"
ng-class="{'vertical-warning-gutters': ctrl.shouldShowVerticalWarningGutters}">
<div class="easel__canvas">
Expand Down
2 changes: 1 addition & 1 deletion kahuna/public/js/image/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ 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' || '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);
Expand Down
4 changes: 2 additions & 2 deletions kahuna/public/js/services/api/media-cropper.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ cropperApi.factory('mediaCropper', ['mediaApi', function(mediaApi) {
return cropperRoot;
}

function createCrop(image, coords, ratio) {
function createCrop(image, coords, ratio, type = 'crop') {
return getCropperRoot().follow('crop').post({
type: 'crop',
type,
source: image.uri,
x: coords.x,
y: coords.y,
Expand Down
3 changes: 2 additions & 1 deletion kahuna/public/js/util/constants/cropOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];

0 comments on commit 7795c33

Please sign in to comment.