Skip to content

Commit

Permalink
An optional CropSpec.rotation field records the rotation which was ap…
Browse files Browse the repository at this point in the history
…plied when these bounds where selected; an EXIF orientation rotation will be explictly shown here.
  • Loading branch information
tonytw1 committed Oct 12, 2024
1 parent 4f1dc89 commit 0a4c37b
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ object MappingTest {
x = 100, y = 100, width = 500, height = 300
),
aspectRatio = Some("5:3"),
`type` = CropExport
`type` = CropExport,
rotation = Some(90),
),
master = Some(testAsset),
assets = List(testAsset)
Expand Down
12 changes: 7 additions & 5 deletions common-lib/src/main/scala/com/gu/mediaservice/model/Crop.scala
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,19 @@ object ExportType {
}


case class CropSpec(uri: String, bounds: Bounds, aspectRatio: Option[String], `type`: ExportType = ExportType.default)
case class CropSpec(uri: String, bounds: Bounds, aspectRatio: Option[String], `type`: ExportType = ExportType.default,
rotation: Option[Int])

object CropSpec {

implicit val cropSpecWrites: Writes[CropSpec] = Json.writes[CropSpec]
implicit val cropSpecReads: Reads[CropSpec] = (
(__ \ "uri").read[String] ~
(__ \ "bounds").read[Bounds] ~
(__ \ "aspectRatio").readNullable[String] ~
(__ \ "type").readNullable[ExportType].map(_.getOrElse(ExportType.default))
)(CropSpec.apply _)
(__ \ "bounds").read[Bounds] ~
(__ \ "aspectRatio").readNullable[String] ~
(__ \ "type").readNullable[ExportType].map(_.getOrElse(ExportType.default)) ~
(__ \ "rotation").readNullable[Int]
)(CropSpec.apply _)
}


Expand Down
2 changes: 1 addition & 1 deletion cropper/app/controllers/CropperController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ class CropperController(auth: Authentication, crops: Crops, store: CropStore, no
// We correct for orientation in the cropper UI; so validate against the oriented dimensions.
orientedDimensions = Seq(apiImage.source.orientedDimensions, apiImage.source.dimensions).flatten.headOption
dimensions <- ifDefined(orientedDimensions, InvalidImage)
cropSpec = ExportRequest.toCropSpec(exportRequest, dimensions)
cropSpec = ExportRequest.toCropSpec(exportRequest, dimensions, apiImage.source.orientationMetadata)
_ <- verify(crops.isWithinImage(cropSpec.bounds, dimensions), InvalidCropRequest)
crop = Crop.createFromCropSource(
by = Some(Authentication.getIdentity(user)),
Expand Down
9 changes: 6 additions & 3 deletions cropper/app/lib/CropSpecMetadata.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import com.gu.mediaservice.model.{Bounds, Crop, CropSpec, Dimensions, ExportType
trait CropSpecMetadata {

def metadataForCrop(crop: Crop, dimensions: Dimensions): Map[String, String] = {
val CropSpec(sourceUri, Bounds(x, y, w, h), r, t) = crop.specification
val CropSpec(sourceUri, Bounds(x, y, w, h), r, t, rotation) = crop.specification
val metadata = Map("source" -> sourceUri,
"bounds-x" -> x,
"bounds-y" -> y,
Expand All @@ -17,7 +17,9 @@ trait CropSpecMetadata {
"date" -> crop.date.map(printDateTime),
"width" -> dimensions.width,
"height" -> dimensions.height,
"aspect-ratio" -> r)
"aspect-ratio" -> r,
"rotation" -> rotation
)

val nonEmptyMetadata = metadata.filter {
case (_, None) => false
Expand All @@ -43,8 +45,9 @@ trait CropSpecMetadata {
h <- getOrElseOrNone(userMetadata, "bounds-height", "bounds_h").map(_.toInt)
ratio = getOrElseOrNone(userMetadata, "aspect-ratio", "aspect_ratio")
exportType = userMetadata.get("type").map(ExportType.valueOf).getOrElse(ExportType.default)
rotation = userMetadata.get("rotation").map(_.toInt)
} yield {
CropSpec(source, Bounds(x, y, w, h), ratio, exportType)
CropSpec(source, Bounds(x, y, w, h), ratio, exportType, rotation)
}
}

Expand Down
2 changes: 1 addition & 1 deletion cropper/app/lib/Crops.scala
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera

def deleteCrops(id: String)(implicit logMarker: LogMarker): Future[Unit] = store.deleteCrops(id)

def dimensionsFromConfig(bounds: Bounds, aspectRatio: Float): List[Dimensions] = if (bounds.isPortrait)
private def dimensionsFromConfig(bounds: Bounds, aspectRatio: Float): List[Dimensions] = if (bounds.isPortrait)
config.portraitCropSizingHeights.filter(_ <= bounds.height).map(h => Dimensions(math.round(h * aspectRatio), h))
else
config.landscapeCropSizingWidths.filter(_ <= bounds.width).map(w => Dimensions(w, math.round(w / aspectRatio)))
Expand Down
28 changes: 16 additions & 12 deletions cropper/app/model/ExportRequest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,22 @@ object ExportRequest {

def boundsFill(dimensions: Dimensions): Bounds = Bounds(0, 0, dimensions.width, dimensions.height)

def toCropSpec(cropRequest: ExportRequest, dimensions: Dimensions): CropSpec = cropRequest match {
case FullExportRequest(uri) =>
CropSpec(
uri,
boundsFill(dimensions),
AspectRatio.calculate(dimensions.width, dimensions.height).map(_.friendly),
FullExport
)
// Map "crop" that covers the whole image to a "full" export
case CropRequest(uri, bounds, ratio) if bounds == boundsFill(dimensions)
=> CropSpec(uri, boundsFill(dimensions), ratio, FullExport)
case CropRequest(uri, bounds, ratio) => CropSpec(uri, bounds, ratio, CropExport)
def toCropSpec(cropRequest: ExportRequest, dimensions: Dimensions, orientationMetadata: Option[OrientationMetadata]): CropSpec = {
val maybeRotation = orientationMetadata.map(_.orientationCorrection())
cropRequest match {
case FullExportRequest(uri) =>
CropSpec(
uri,
boundsFill(dimensions),
AspectRatio.calculate(dimensions.width, dimensions.height).map(_.friendly),
FullExport,
rotation = maybeRotation
)
// Map "crop" that covers the whole image to a "full" export
case CropRequest(uri, bounds, ratio) if bounds == boundsFill(dimensions)
=> CropSpec(uri, boundsFill(dimensions), ratio, FullExport, rotation = maybeRotation)
case CropRequest(uri, bounds, ratio) => CropSpec(uri, bounds, ratio, CropExport, rotation = maybeRotation)
}
}

}
7 changes: 5 additions & 2 deletions cropper/test/lib/CropSpecMetadataTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ class CropSpecMetadataTest extends AnyFunSpec with Matchers with CropSpecMetadat

private val cropSpec = CropSpec(
uri = "/test",
bounds = Bounds(1, 2, 3, 4), aspectRatio = Some("16:9"), `type` = ExportType.default
bounds = Bounds(1, 2, 3, 4), aspectRatio = Some("16:9")
, `type` = ExportType.default, rotation = Some(90)
)
private val crop = Crop(
id = Some("123"),
Expand All @@ -29,15 +30,17 @@ class CropSpecMetadataTest extends AnyFunSpec with Matchers with CropSpecMetadat
metadata("height") shouldBe "480"
metadata.get("aspect-ratio") shouldBe Some("16:9")
metadata.get("author") shouldBe Some("Tony McCrae")
metadata.get("rotation") shouldBe Some("90")
}

it("should handle empty optional fields") {
val withEmptyField = cropSpec.copy(aspectRatio = None)
val withEmptyField = cropSpec.copy(aspectRatio = None, rotation = None)

val metadata = metadataForCrop(crop.copy(specification = withEmptyField, author = None), dimensions)

metadata.get("aspect-ratio") shouldBe None
metadata.get("author") shouldBe None
metadata.get("rotation") shouldBe None
}

it("should round trip metadata back to crop spec") {
Expand Down
2 changes: 1 addition & 1 deletion media-api/test/lib/ImageResponseTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class ImageResponseTest extends AnyFunSpec with Matchers with Fixtures {

import TestUtils._

val testCrop = Crop(Some("crop-id"), None, None, CropSpec("test-uri", Bounds(0, 0, 0, 0), None), None, Nil)
val testCrop = Crop(Some("crop-id"), None, None, CropSpec("test-uri", Bounds(0, 0, 0, 0), None, rotation = None), None, Nil)
val testUsage = Usage(id = "usage-id", references = Nil, platform = PrintUsage, media = "test", status = PendingUsageStatus, dateAdded = None, dateRemoved = None, now())

val imgWithNoExportsAndUsages = img
Expand Down
2 changes: 1 addition & 1 deletion thrall/test/helpers/Fixtures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ trait Fixtures {
def imageWithPhotoshoot(photoshoot: Photoshoot): Image = createImage(id = UUID.randomUUID().toString, StaffPhotographer("Tom Jenkins", "The Guardian"), optPhotoshoot = Some(photoshoot))

def crop = {
val cropSpec = CropSpec("/test", Bounds(0,0,0,0), None)
val cropSpec = CropSpec("/test", Bounds(0,0,0,0), None, rotation = None)
Crop(None, None, None, cropSpec: CropSpec, None, List.empty)
}

Expand Down

0 comments on commit 0a4c37b

Please sign in to comment.