diff --git a/article/app/model/structuredData/Image.scala b/article/app/model/structuredData/Image.scala
index 3d252000a879..49e8a5b83b5e 100644
--- a/article/app/model/structuredData/Image.scala
+++ b/article/app/model/structuredData/Image.scala
@@ -1,18 +1,21 @@
package model.structuredData
-import model.{ImageAsset, ImageElement}
-import play.api.libs.json.{JsString, JsValue, Json}
+import model.ImageElement
+import play.api.libs.json.{JsValue, Json}
import views.support.Item700
object Image {
def apply(picture: ImageElement): JsValue = {
- val asset: Option[ImageAsset] = Item700.bestFor(picture.images)
+ val url = Item700.bestSrcFor(picture.images).getOrElse("")
+ val height = Item700.trueHeightFor(picture.images).getOrElse(0)
+ val width = Item700.trueWidthFor(picture.images).getOrElse(0)
+
Json.obj(
"@type" -> "ImageObject",
- "url" -> JsString(Item700.bestSrcFor(picture.images).getOrElse("")),
- "height" -> asset.fold(0)(_.height),
- "width" -> asset.fold(0)(_.width)
+ "url" -> url,
+ "height" -> height,
+ "width" -> width
)
}
diff --git a/common/app/views/fragments/imageFigure.scala.html b/common/app/views/fragments/imageFigure.scala.html
index 41a7b8c88517..2fdc5e6095b8 100644
--- a/common/app/views/fragments/imageFigure.scala.html
+++ b/common/app/views/fragments/imageFigure.scala.html
@@ -38,11 +38,9 @@
}
- @defining(Item700.bestFor(picture)) { asset =>
-
-
-
- }
+
+
+
@defining(
if (isMain) {
diff --git a/common/app/views/support/Profile.scala b/common/app/views/support/Profile.scala
index 6754b2ae44f3..a77778ac0b0c 100644
--- a/common/app/views/support/Profile.scala
+++ b/common/app/views/support/Profile.scala
@@ -43,18 +43,58 @@ sealed trait ElementProfile {
def altTextFor(image: ImageMedia): Option[String] =
bestFor(image).flatMap(_.altText)
+ def trueWidthFor(image: ImageMedia): Option[Int] = {
+ val maybeAssetWidth: Option[Int] = bestFor(image).map(_.width)
+ val maybeTrueProfileWidth: Option[Int] = width.map { pixels =>
+ Math.round(pixels * dpr.getOrElse(1d)).toInt
+ }
+
+ // If the asset and profile have valid dimensions then take the minimum (due to fit=max)
+ val resizedDimension = for {
+ assetWidth <- maybeAssetWidth
+ profileWidth <- maybeTrueProfileWidth
+ } yield Math.min(assetWidth, profileWidth)
+
+ if(isCropped && ImageServerSwitch.isSwitchedOn) maybeTrueProfileWidth
+ else if(ImageServerSwitch.isSwitchedOn) resizedDimension.orElse(maybeAssetWidth).orElse(maybeTrueProfileWidth)
+ else maybeAssetWidth
+ }
+
+ def trueHeightFor(image: ImageMedia): Option[Int] = {
+ val best = bestFor(image)
+ val maybeTrueProfileHeight: Option[Int] = height.map { pixels =>
+ Math.round(pixels * dpr.getOrElse(1d)).toInt
+ }
+
+ if(isCropped) {
+ maybeTrueProfileHeight
+ } else {
+ // If we are fit=max then the aspect ratio is maintained, so use the true width
+ for {
+ height <- best.map(_.height)
+ width <- best.map(_.width)
+ trueWidth <- trueWidthFor(image)
+ if height != 0
+ } yield (trueWidth * height) / width
+ }
+ }
+
+ val dpr: Option[Double] = if (hidpi) {
+ if (isPng) {
+ Some(1.3)
+ } else {
+ Some(2)
+ }
+ } else None
+
+ lazy val isCropped = fitParam.endsWith("=crop")
+
// NOTE - if you modify this in any way there is a decent chance that you decache all our images :(
val qualityparam = if (hidpi) {"q=20"} else {"q=55"}
val autoParam = if (autoFormat) "auto=format" else ""
val sharpParam = "usm=12"
val fitParam = "fit=max"
- val dprParam = if (hidpi) {
- if (isPng) {
- "dpr=1.3"
- } else {
- "dpr=2"
- }
- } else {""}
+ val dprParam = dpr.map("dpr=" + _.toString).getOrElse("")
val heightParam = height.map(pixels => s"h=$pixels").getOrElse("")
val widthParam = width.map(pixels => s"w=$pixels").getOrElse("")
diff --git a/common/test/views/support/ProfileTest.scala b/common/test/views/support/ProfileTest.scala
new file mode 100644
index 000000000000..d37521afbc2e
--- /dev/null
+++ b/common/test/views/support/ProfileTest.scala
@@ -0,0 +1,56 @@
+package views.support
+
+import com.gu.contentapi.client.model.v1.AssetType
+import conf.switches.Switches._
+import model.{ImageAsset, ImageMedia}
+import org.scalatest.{BeforeAndAfter, FlatSpec, Matchers}
+
+class ProfileTest extends FlatSpec with Matchers with BeforeAndAfter {
+
+ before {
+ ImageServerSwitch.switchOn()
+ }
+
+ after {
+ ImageServerSwitch.switchOn()
+ }
+
+ object HidpiProfileSmall extends Profile(width = Some(100), height = Some(60), hidpi = true)
+
+ val imageAsset: ImageAsset = ImageAsset(
+ fields = Map("width" -> "460", "height" -> "276"),
+ mediaType = AssetType.Image.name,
+ mimeType = Some("image/jpeg"),
+ url = Some("https://static.guim.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.jpeg")
+ )
+ val image: ImageMedia = ImageMedia.apply(Seq(imageAsset))
+
+ "A hidpi profile with small width" should "return double the dimensions of the profile for the true dimensions" in {
+ HidpiProfileSmall.trueWidthFor(image) should be(Some(200))
+ HidpiProfileSmall.trueHeightFor(image) should be(Some(120))
+ }
+
+ object HidpiProfileLarge extends Profile(width = Some(1000), height = Some(600), hidpi = true)
+
+ "A hidpi profile with large width" should "return the dimensions of the original asset" in {
+ HidpiProfileLarge.trueWidthFor(image) should be(Some(460))
+ HidpiProfileLarge.trueHeightFor(image) should be(Some(276))
+ }
+
+ "A non hidpi profile with small width" should "return the dimensions of the profile" in {
+ Item120.trueWidthFor(image) should be(Some(120))
+ Item120.trueHeightFor(image) should be(Some(72))
+ }
+
+ "A non hidpi profile with large width" should "return the dimensions of the original asset" in {
+ Item1200.trueWidthFor(image) should be(Some(460))
+ Item1200.trueHeightFor(image) should be(Some(276))
+ }
+
+
+ "A non hidpi profile with small width and image resizing off" should "return the dimensions of the original asset" in {
+ ImageServerSwitch.switchOff()
+ Item120.trueWidthFor(image) should be(Some(460))
+ Item120.trueHeightFor(image) should be(Some(276))
+ }
+}