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 for issue #628 #118

Closed
wants to merge 5 commits into
base: dev
from

Conversation

Projects
None yet
2 participants
@horst-n
Contributor

horst-n commented Aug 14, 2018

changed from the old behaviour to now return a small variation that fit into aspect ratio of the target dimensions.

@ryancramerdesign I have tested this thouroughful and it seems to work with all cases:

  • No dimension to small; One dimension to small; Two dimension to small;
  • Requesting a landscape and source is: landscape; portrait; square;
  • Requesting a portrait and source is: landscape; portrait; square;
  • Requesting a square and source is: landscape; portrait; square;
    All with enabled / disabled $config->imageSizerOptions['cropping']
fix for issue #628
changed from the old behaviour to now return a small variation that fit into aspect ratio of the target dimensions.

@ryancramerdesign I have tested this thouroughful and it seems to work with all cases:
No dimension to small; One dimension to small; Two dimension to small;
Requesting a landscape and source is: landscape; portrait; square;
Requesting a portrait and source is: landscape; portrait; square;
Requesting a square and source is: landscape; portrait; square;

horst-n added some commits Aug 19, 2018

added support for sharpening to the new behaviour
With the old behaviour, the $resizeMethod everytime resulted in 0 for images that were to small when $upscaling was disabled.
Now, with the new behaviour, it also can result in 4, but without resizing, only with cropping. Therefore we have to disable sharpening here at this point too.

For example, see: https://github.com/processwire/processwire/blob/5554e87b4732729926b274d4111b4eec3a559a0d/wire/core/ImageSizerEngineGD.php#L211-L232
removed obsolete comments, related to focus & zoom
This was already implemented in the ImageSizerEngine.php with this commit: 5493d5f
@horst-n

This comment has been minimized.

Show comment
Hide comment
@horst-n

horst-n Aug 19, 2018

Contributor

@ryancramerdesign
Hi Ryan, within the last days, where I have used the new default bevaviour, I encountered that the change was a bit more obstrusive as I thought first. But now, after adding support for sharpening, all aspects are covered.

I also cleaned up some parts in the ImageSizerEngineGD.php for better reading and understanding.

Contributor

horst-n commented Aug 19, 2018

@ryancramerdesign
Hi Ryan, within the last days, where I have used the new default bevaviour, I encountered that the change was a bit more obstrusive as I thought first. But now, after adding support for sharpening, all aspects are covered.

I also cleaned up some parts in the ImageSizerEngineGD.php for better reading and understanding.

@ryancramerdesign

This comment has been minimized.

Show comment
Hide comment
@ryancramerdesign

ryancramerdesign Sep 12, 2018

Contributor

Thanks @horst-n this is great! I am adding this today.

Contributor

ryancramerdesign commented Sep 12, 2018

Thanks @horst-n this is great! I am adding this today.

ryancramerdesign added a commit that referenced this pull request Sep 12, 2018

@horst-n

This comment has been minimized.

Show comment
Hide comment
@horst-n

horst-n Sep 13, 2018

Contributor

Thanks Ryan!

Contributor

horst-n commented Sep 13, 2018

Thanks Ryan!

@horst-n horst-n closed this Sep 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment