search

caffeinetv / snappy

Image processing service
Apache License 2.0
3 stars 1 forks source link

resize transformations mapping definition #5

Closed joarleymoraes closed 7 years ago

joarleymoraes commented 7 years ago

Based on the followiing Wiki I wrote:

https://github.com/caffeinetv/snappy/wiki/Transformations-Mapping-Imgix:Fastly#resize

You can see that almost all resulting transformations from Imgix and Fastly do not match. We need to define which mapping we are going to use in our API.

I'm going to continue to create the mapping for the other operations as well, and create separate issues.

pas256 commented 7 years ago

Hey @nicolasartman, @joarleymoraes has noticed several differences between Imgix and Fastly, and I wanted to get your input on which way to go. It seems like is most cases, Fastly is doing the preferred thing. What do you think?

nicolasartman commented 7 years ago

@joarleymoraes thanks for highlighting these! We can go with the fastly behavior in all the documented cases. It doesn't look to me like fastly actually has fit=clip, which means that imgix's fit=clip and fastly's fit=bounds are equivalent I think. If we can just map them to the same behavior (II) that'd be great.

In the docs, it looks to me like q | quality and fm | format are switched (the explanation for q talks about formats and fm talks about quality).

Also, in case it wasn't stated, I'd to support both fastly's query param names and imgix's (e.g. w and width)

joarleymoraes commented 7 years ago

@nicolasartman You are right, Fastly does not have fit=clip, so it just resizes as if fit wasn't provided, which is (I). So if I understand you correctly, in our API, we should map fit=clip to the behavior described in (II).

Also fixed the quality/format swap.

Both full name and aliases params will be supported.

joarleymoraes commented 7 years ago

If fit=clip should be (II), then it will behave like fit=bounds. So we just need one of them. Which one should it be ? @pas256 @nicolasartman

Another option is to make fit=clip behave as (I).

joarleymoraes commented 7 years ago

I have added to the Wiki the mapping our API:

https://github.com/caffeinetv/snappy/wiki/Transformations-Mapping-Imgix:Fastly:Snappy

joarleymoraes commented 7 years ago

I have implemented fit=clip as (I)

nicolasartman commented 7 years ago

@joarleymoraes fit=clip should act the same as fit=bounds. We'll need to support it for a while for compatibility with old iphone app clients that are using imgix right now, but also support it for new fastly clients that use clip=bounds. So that makes them both identical behavior equal to (II)

joarleymoraes commented 7 years ago

Got it.