Replace Round function with Ceil in fit method (Black line)

[lastonoga]

There is a problem when fitWidth = 100.234. Round method make fitWidth equals to 100, but it should be 101 (to prevent black line appearing). #249

I don't know Go and how to run tests, so if you have ability to write tests, please, do it.

[Dynom]

Not too unsurprisingly, two tests fail because of this change. The question is, what is correct:

--- FAIL: TestCalculateDestinationFitDimension (0.00s)
    image_test.go:104: Fit dimensions calculation failure
        Expected : 710/555 (width/height)
        Actual   : 710/556 (width/height)
        {imageWidth:1279 imageHeight:1000 optionWidth:710 optionHeight:9999 fitWidth:710 fitHeight:555}
    image_test.go:104: Fit dimensions calculation failure
        Expected : 312/173 (width/height)
        Actual   : 312/174 (width/height)
        {imageWidth:900 imageHeight:500 optionWidth:312 optionHeight:312 fitWidth:312 fitHeight:173}

/cc https://github.com/h2non/imaginary/issues/240#issuecomment-458041205

[lastonoga]

@Dynom Ceil should be only for fitWidth var. I changed it.

[lastonoga]

We need to add new test

imageWidth: 851
imageHeight: 1024

fitWidth: 3000
fitHeight: 100

Expected : 83/100 (width/height)
Actual   : 84/100 (width/height) 

So ceil only for fitWidth will crash some vertical images. Looks like we need to change algorithm. Or do something like this 😂 fitWidth -= 0.2

There is a problem that we can't have "half of the pixel", so i think there are 2 ways:

1. Do hack like fitWidth -= 0.2
2. Make image a little bit bigger where dimension is the best fitted (where Round works fine)

[Dynom]

When I look at the implementation in libvips, it's way more complicated and contains many more decision branches (including: https://github.com/libvips/libvips/blob/1a836052385c220ad32f9ce0c9a3363f1f919ad6/libvips/arithmetic/max.c#L534).

I think we should proceed with leaning more on vips_thumbnail_buffer() (see comment). That would take care of the resize/resample/fit/crop and general scaling.

[evrenkutar]

@lastonoga hi, @Dynom hi, we are using this app as a fork in our company and had the same issue, i have applied lastonoga's fix to our repo. i just wanted to let you know that i do this and thank you for the solution. when you merge this i will reset and update from the repo. have a nice day

[Dynom]

@evrenkutar the fix isn't "correct" for all situations, which is why we haven't merged it in yet. Great if it works for your situation, but be careful :-)