Downscale filter scales an image to fit bounding box

[aminin]

liip_imagine:
    filter_sets:
        my_downscale:
            filters:
                downscale: { max: [400,400] }  # Transforms 800x1600 to 200x400 rather than 400x800

[lsmith77]

that seems like a BC break .. I fear we would need to introduce a new config option for this new behavior

[aminin]

@lsmith77

In this PR I propose the downscale filter to work as downscale to fit bounding box
The actual behaviour is downscale to get out of the bounding box and sometimes upscale instead of downscale

It is hard to assume that the second one is the supposed behaviour cause it's oviously incorrect.

I see no BC break here.

[lsmith77]

yeah classic bug fix vs. BC break ..

the current filter has existed since Jan 2015 introduced in #610 by @sascha-meissner

BTW the PR contained a test case which however didn't expose this bug:
https://github.com/liip/LiipImagineBundle/blob/master/Tests/Imagine/Filter/Loader/FloatToIntCastByRoundDownscaleFilterLoaderTest.php

[aminin]

Hi, @lsmith77 .

Thank you for the explanation of historical background.

So, we have 2 choices:

1. If you insist, I'd

  # introduce a new config option mode: { choice: inset|incorrect; default: incorrect }
  downscale: { max: [400, 400], mode: inset }

2. if you not opposed, we merge this PR as is, assuming that

• no failing tests means no BC break
• nobody cares about downscale because it's incorrect and has a working alternative thumbnail
• if there is an implicit but significant case when BC break really occurs, someone will report that

[lsmith77]

you convinced me .. lets merge. thx! :)