Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AreaWidth calculation #250

Merged
merged 1 commit into from
Dec 5, 2018
Merged

Fix AreaWidth calculation #250

merged 1 commit into from
Dec 5, 2018

Conversation

fisherking
Copy link

Fixes #249

@Dynom
Copy link
Collaborator

Dynom commented Nov 2, 2018

Excellent, thanks!

I would like to see a test-case to prove the fix. We're slightly behind on coverage and this is an excellent candidate to improve it for that function. I've quickly wrote one you could include in resizer_test.go. It requires a small change to resizer.go as well, I've included a patch you could apply to make it all work.

@h2non Something we want to merge back soon I suppose :-)

diff --git a/resizer.go b/resizer.go
index 98a95ae..934b937 100644
--- a/resizer.go
+++ b/resizer.go
@@ -12,6 +12,10 @@ import (
 	"math"
 )

+var (
+	ErrExtractAreaParamsRequired = errors.New("extract area width/height params are required")
+)
+
 // resizer is used to transform a given image as byte buffer
 // with the passed options.
 func resizer(buf []byte, o Options) ([]byte, error) {
@@ -279,7 +283,7 @@ func extractOrEmbedImage(image *C.VipsImage, o Options) (*C.VipsImage, error) {
 			o.AreaHeight = o.Height
 		}
 		if o.AreaWidth == 0 || o.AreaHeight == 0 {
-			return nil, errors.New("Extract area width/height params are required")
+			return nil, ErrExtractAreaParamsRequired
 		}
 		image, err = vipsExtract(image, o.Left, o.Top, o.AreaWidth, o.AreaHeight)
 		break
diff --git a/resizer_test.go b/resizer_test.go
index a4b54b0..6a116a2 100644
--- a/resizer_test.go
+++ b/resizer_test.go
@@ -364,6 +364,54 @@ func TestExtractCustomAxis(t *testing.T) {
 	Write("testdata/test_extract_custom_axis_out.jpg", newImg)
 }

+func TestExtractOrEmbedImage(t *testing.T) {
+	buf, _ := Read("testdata/test.jpg")
+	input, _, err := loadImage(buf)
+	if err != nil {
+		t.Fatalf("Unable to load image %s", err)
+	}
+
+	o := Options{
+		Top:    10,
+		Left:   10,
+		Width:  100,
+		Height: 200,
+
+		// Fields to test
+		AreaHeight: 0,
+		AreaWidth:  0,
+
+		Quality: 100, /* Needs a value to satisfy libvips */
+	}
+
+	result, err := extractOrEmbedImage(input, o)
+	if err != nil {
+		if err == ErrExtractAreaParamsRequired {
+			t.Fatalf("Expecting AreaWidth and AreaHeight to have been defined")
+		}
+
+		t.Fatalf("Unknown error occurred %s", err)
+	}
+
+	image, err := saveImage(result, o)
+	if err != nil {
+		t.Fatalf("Failed saving image %s", err)
+	}
+
+	test, err := Size(image)
+	if err != nil {
+		t.Fatalf("Failed fetching the size %s", err)
+	}
+
+	if test.Height != o.Height {
+		t.Errorf("Extract failed, resulting Height %d doesn't match %d", test.Height, o.Height)
+	}
+
+	if test.Width != o.Width {
+		t.Errorf("Extract failed, resulting Width %d doesn't match %d", test.Width, o.Width)
+	}
+}
+
 func TestConvert(t *testing.T) {
 	width, height := 300, 240
 	formats := [3]ImageType{PNG, WEBP, JPEG}

@h2non h2non merged commit a40dadf into h2non:master Dec 5, 2018
Dynom added a commit that referenced this pull request Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants