Replace Round function with Ceil in fit method (Black line)#250
Replace Round function with Ceil in fit method (Black line)#250lastonoga wants to merge 2 commits intoh2non:masterfrom
Conversation
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)
|
Not too unsurprisingly, two tests fail because of this change. The question is, what is correct: /cc #240 (comment) |
|
@Dynom Ceil should be only for fitWidth var. I changed it. |
|
We need to add new test So ceil only for fitWidth will crash some vertical images. Looks like we need to change algorithm. There is a problem that we can't have "half of the pixel", so i think there are 2 ways:
|
|
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. |
|
@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 |
|
@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 :-) |
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.