Optimal and correct DPR support

process.go

@@ -134,27 +134,41 @@ func extractMeta(img *C.VipsImage) (int, int, int, bool) {
 	return width, height, angle, flip
 }
 
-func needToScale(width, height int, po *processingOptions) bool {
-	return ((po.Width != 0 && po.Width != width) || (po.Height != 0 && po.Height != height)) &&
-		(po.Resize == resizeFill || po.Resize == resizeFit)
-}
-
 func calcScale(width, height int, po *processingOptions) float64 {
-	srcW, srcH := float64(width), float64(height)
+	// If we're going only to crop, we need only to scale down to DPR.
-
+	// Scaling up while cropping is not optimal on this stage, we'll do it later if needed.
-	wr := float64(po.Width) / srcW
+	if po.Resize == resizeCrop {
-	hr := float64(po.Height) / srcH
+		if po.Dpr < 1 {
+			return po.Dpr
+		}
+		return 1
+	}
 
 	var scale float64
 
-	if po.Width == 0 {
+	srcW, srcH := float64(width), float64(height)
-		scale = hr
+
-	} else if po.Height == 0 {
+	if (po.Width == 0 || po.Width == width) && (po.Height == 0 || po.Height == height) {
-		scale = wr
+		scale = 1
-	} else if po.Resize == resizeFit {
-		scale = math.Min(wr, hr)
 	} else {
-		scale = math.Max(wr, hr)
+		wr := float64(po.Width) / srcW
+		hr := float64(po.Height) / srcH
+
+		if po.Width == 0 {
+			scale = hr
+		} else if po.Height == 0 {
+			scale = wr
+		} else if po.Resize == resizeFit {
+			scale = math.Min(wr, hr)
+		} else {
+			scale = math.Max(wr, hr)
+		}
+	}
+
+	scale = scale * po.Dpr
+
+	if !po.Enlarge && scale > 1 {
+		return 1
 	}
 
 	if srcW*scale < 1 {
@@ -188,67 +202,73 @@ func calcShink(scale float64, imgtype imageType) int {
 	return 1
 }
 
-func calcCrop(width, height int, po *processingOptions) (left, top int) {
+func calcCrop(width, height, cropWidth, cropHeight int, gravity *gravityOptions) (left, top int) {
-	if po.Gravity.Type == gravityFocusPoint {
+	if gravity.Type == gravityFocusPoint {
-		pointX := int(float64(width) * po.Gravity.X)
+		pointX := int(float64(width) * gravity.X)
-		pointY := int(float64(height) * po.Gravity.Y)
+		pointY := int(float64(height) * gravity.Y)
 
-		left = maxInt(0, minInt(pointX-po.Width/2, width-po.Width))
+		left = maxInt(0, minInt(pointX-cropWidth/2, width-cropWidth))
-		top = maxInt(0, minInt(pointY-po.Height/2, height-po.Height))
+		top = maxInt(0, minInt(pointY-cropHeight/2, height-cropHeight))
 
 		return
 	}
 
-	left = (width - po.Width + 1) / 2
+	left = (width - cropWidth + 1) / 2
-	top = (height - po.Height + 1) / 2
+	top = (height - cropHeight + 1) / 2
 
-	if po.Gravity.Type == gravityNorth || po.Gravity.Type == gravityNorthEast || po.Gravity.Type == gravityNorthWest {
+	if gravity.Type == gravityNorth || gravity.Type == gravityNorthEast || gravity.Type == gravityNorthWest {
 		top = 0
 	}
 
-	if po.Gravity.Type == gravityEast || po.Gravity.Type == gravityNorthEast || po.Gravity.Type == gravitySouthEast {
+	if gravity.Type == gravityEast || gravity.Type == gravityNorthEast || gravity.Type == gravitySouthEast {
-		left = width - po.Width
+		left = width - cropWidth
 	}
 
-	if po.Gravity.Type == gravitySouth || po.Gravity.Type == gravitySouthEast || po.Gravity.Type == gravitySouthWest {
+	if gravity.Type == gravitySouth || gravity.Type == gravitySouthEast || gravity.Type == gravitySouthWest {
-		top = height - po.Height
+		top = height - cropHeight
 	}
 
-	if po.Gravity.Type == gravityWest || po.Gravity.Type == gravityNorthWest || po.Gravity.Type == gravitySouthWest {
+	if gravity.Type == gravityWest || gravity.Type == gravityNorthWest || gravity.Type == gravitySouthWest {
 		left = 0
 	}
 
 	return
 }
 
+func resizeImage(img **C.struct__VipsImage, scale float64, hasAlpha bool) error {
+	var err error
+
+	premultiplied := false
+	var bandFormat C.VipsBandFormat
+
+	if hasAlpha {
+		if bandFormat, err = vipsPremultiply(img); err != nil {
+			return err
+		}
+		premultiplied = true
+	}
+
+	if err = vipsResize(img, scale); err != nil {
+		return err
+	}
+
+	if premultiplied {
+		if err = vipsUnpremultiply(img, bandFormat); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func transformImage(ctx context.Context, img **C.struct__VipsImage, data []byte, po *processingOptions, imgtype imageType) error {
 	var err error
- 
-	imgWidth, imgHeight, angle, flip := extractMeta(*img)
-	if po.Dpr != 1 {
-		if po.Width != 0 {
-			po.Width = int(float32(po.Width) * po.Dpr)
-		} 
-		if po.Height != 0 {
-			po.Height = int(float32(po.Height) * po.Dpr)
-		}
-	}
-	// Ensure we won't crop out of bounds
-	if !po.Enlarge || po.Resize == resizeCrop {
-		if imgWidth < po.Width {
-			po.Width = imgWidth
-		}
 
-		if imgHeight < po.Height {
+	imgWidth, imgHeight, angle, flip := extractMeta(*img)
-			po.Height = imgHeight
-		}
-	}
 
 	hasAlpha := vipsImageHasAlpha(*img)
 
-	if needToScale(imgWidth, imgHeight, po) {
+	if scale := calcScale(imgWidth, imgHeight, po); scale != 1 {
-		scale := calcScale(imgWidth, imgHeight, po)
-
 		// Do some shrink-on-load
 		if scale < 1.0 && data != nil {
 			if shrink := calcShink(scale, imgtype); shrink != 1 {
@@ -262,28 +282,12 @@ func transformImage(ctx context.Context, img **C.struct__VipsImage, data []byte,
 			}
 		}
 
-		premultiplied := false
+		if err = resizeImage(img, scale, hasAlpha); err != nil {
-		var bandFormat C.VipsBandFormat
-
-		if hasAlpha {
-			if bandFormat, err = vipsPremultiply(img); err != nil {
-				return err
-			}
-			premultiplied = true
-		}
-
-		if err = vipsResize(img, scale); err != nil {
 			return err
 		}
 
 		// Update actual image size after resize
 		imgWidth, imgHeight, _, _ = extractMeta(*img)
-
-		if premultiplied {
-			if err = vipsUnpremultiply(img, bandFormat); err != nil {
-				return err
-			}
-		}
 	}
 
 	if err = vipsImportColourProfile(img); err != nil {
@@ -312,20 +316,31 @@ func transformImage(ctx context.Context, img **C.struct__VipsImage, data []byte,
 
 	checkTimeout(ctx)
 
-	if po.Width == 0 {
+	cropW, cropH := po.Width, po.Height
-		po.Width = imgWidth
+
+	if po.Dpr < 1 || (po.Dpr > 1 && po.Resize != resizeCrop) {
+		cropW = int(float64(cropW) * po.Dpr)
+		cropH = int(float64(cropH) * po.Dpr)
 	}
 
-	if po.Height == 0 {
+	if cropW == 0 {
-		po.Height = imgHeight
+		cropW = imgWidth
+	} else {
+		cropW = minInt(cropW, imgWidth)
 	}
 
-	if po.Width < imgWidth || po.Height < imgHeight {
+	if cropH == 0 {
+		cropH = imgHeight
+	} else {
+		cropH = minInt(cropH, imgHeight)
+	}
+
+	if cropW < imgWidth || cropH < imgHeight {
 		if po.Gravity.Type == gravitySmart {
 			if err = vipsImageCopyMemory(img); err != nil {
 				return err
 			}
-			if err = vipsSmartCrop(img, po.Width, po.Height); err != nil {
+			if err = vipsSmartCrop(img, cropW, cropH); err != nil {
 				return err
 			}
 			// Applying additional modifications after smart crop causes SIGSEGV on Alpine
@@ -334,8 +349,8 @@ func transformImage(ctx context.Context, img **C.struct__VipsImage, data []byte,
 				return err
 			}
 		} else {
-			left, top := calcCrop(imgWidth, imgHeight, po)
+			left, top := calcCrop(imgWidth, imgHeight, cropW, cropH, &po.Gravity)
-			if err = vipsCrop(img, left, top, po.Width, po.Height); err != nil {
+			if err = vipsCrop(img, left, top, cropW, cropH); err != nil {
 				return err
 			}
 		}
@@ -343,6 +358,16 @@ func transformImage(ctx context.Context, img **C.struct__VipsImage, data []byte,
 		checkTimeout(ctx)
 	}
 
+	if po.Enlarge && po.Resize == resizeCrop && po.Dpr > 1 {
+		// We didn't enlarge the image before, because is wasn't optimal. Now it's time to do it
+		if err = resizeImage(img, po.Dpr, hasAlpha); err != nil {
+			return err
+		}
+		if err = vipsImageCopyMemory(img); err != nil {
+			return err
+		}
+	}
+
 	if hasAlpha && (po.Flatten || po.Format == imageTypeJPEG) {
 		if err = vipsFlatten(img, po.Background); err != nil {
 			return err

processing_options.go

@@ -117,13 +117,13 @@ type processingOptions struct {
 	Resize     resizeType
 	Width      int
 	Height     int
+	Dpr        float64
 	Gravity    gravityOptions
 	Enlarge    bool
 	Format     imageType
 	Quality    int
 	Flatten    bool
 	Background color
-	Dpr        float32
 	Blur       float32
 	Sharpen    float32
 
@@ -140,6 +140,7 @@ const (
 	imageURLCtxKey          = ctxKey("imageUrl")
 	processingOptionsCtxKey = ctxKey("processingOptions")
 	urlTokenPlain           = "plain"
+	maxClientHintDPR        = 8
 )
 
 var (
@@ -374,6 +375,20 @@ func applyResizeOption(po *processingOptions, args []string) error {
 	return nil
 }
 
+func applyDprOption(po *processingOptions, args []string) error {
+	if len(args) > 1 {
+		return fmt.Errorf("Invalid dpr arguments: %v", args)
+	}
+
+	if d, err := strconv.ParseFloat(args[0], 64); err == nil || (d > 0 && d != 1) {
+		po.Dpr = d
+	} else {
+		return fmt.Errorf("Invalid dpr: %s", args[0])
+	}
+
+	return nil
+}
+
 func applyGravityOption(po *processingOptions, args []string) error {
 	if g, ok := gravityTypes[args[0]]; ok {
 		po.Gravity.Type = g
@@ -472,19 +487,6 @@ func applyBlurOption(po *processingOptions, args []string) error {
 	return nil
 }
 
-func applyDprOption(po *processingOptions, args []string) error {
-	if len(args) > 1 {
-		return fmt.Errorf("Invalid dpr arguments: %v", args)
-	}
-
-	if d, err := strconv.ParseFloat(args[0], 32); err == nil || (d > 0 && d != 1) {
-		po.Dpr = float32(d)
-	} else {
-		return fmt.Errorf("Invalid dpr: %s", args[0])
-	}
-
-	return nil
-}
 func applySharpenOption(po *processingOptions, args []string) error {
 	if len(args) > 1 {
 		return fmt.Errorf("Invalid sharpen arguments: %v", args)
@@ -631,6 +633,10 @@ func applyProcessingOption(po *processingOptions, name string, args []string) er
 		if err := applyEnlargeOption(po, args); err != nil {
 			return err
 		}
+	case "dpr":
+		if err := applyDprOption(po, args); err != nil {
+			return err
+		}
 	case "gravity", "g":
 		if err := applyGravityOption(po, args); err != nil {
 			return err
@@ -663,10 +669,6 @@ func applyProcessingOption(po *processingOptions, name string, args []string) er
 		if err := applyCacheBusterOption(po, args); err != nil {
 			return err
 		}
-	case "dpr":
-		if err := applyDprOption(po, args); err != nil {
-			return err
-		}
 	default:
 		return fmt.Errorf("Unknown processing option: %s", name)
 	}
@@ -743,8 +745,8 @@ func defaultProcessingOptions(headers *processingHeaders) (*processingOptions, e
 		}
 	}
 	if conf.EnableClientHints && len(headers.DPR) > 0 {
-		if dpr, err := strconv.ParseFloat(headers.DPR, 32); err == nil || (dpr > 0 && dpr <= 8) {
+		if dpr, err := strconv.ParseFloat(headers.DPR, 64); err == nil || (dpr > 0 && dpr <= maxClientHintDPR) {
-			po.Dpr = float32(dpr)
+			po.Dpr = dpr
 		}
 	}
 	if _, ok := conf.Presets["default"]; ok {