Skip to content
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

Image Resampling configuration #21

Closed
ColmMcBarron opened this issue May 16, 2017 · 11 comments
Closed

Image Resampling configuration #21

ColmMcBarron opened this issue May 16, 2017 · 11 comments

Comments

@ColmMcBarron
Copy link

In the docs:
https://docs.silverstripe.org/en/4/developer_guides/files/images/

It says that the images will not be resampled by default, however the code does resample by default (private static $force_resample = true;) in the ImageManipulation trait.

So if you upload an already compressed image, it looks quite bad, so I changed the option in the YAML file to correct this issue for now.

@dhensby
Copy link
Contributor

dhensby commented Jun 26, 2017

@tractorcow do you know if this is intended? should we turn off force_resample or amend the docs?

@tractorcow
Copy link
Contributor

Oh, the docs are out of date. resample by default is on intentionally.

@dhensby
Copy link
Contributor

dhensby commented Jun 27, 2017

ok, thanks. I'll update the docs.

@dhensby
Copy link
Contributor

dhensby commented Jun 27, 2017

Can you provide me some context as to why it's on so the docs can explain that and possibly highlight why people may not wish to change this setting?

@tractorcow
Copy link
Contributor

The new preference by default is performance over quality. Large images will stop slowing down pages if left un-sampled. I believe @jonom was the driver behind this.

@jonom
Copy link

jonom commented Jun 28, 2017

Yeah here is the commit: silverstripe/silverstripe-framework@40cc567

Basic idea is that since SilverStripe handles all your image manipulations and compresses them for output, you should be aiming to upload hiqh quality (minimal or no compression) source images to your assets store, so you're not compressing images twice over and getting a messy result.

If you do follow that practice, you don't want to send out the source images to the client as they will be much larger than they need to be. SilverStripe previously skipped resampling and sent out the original image if it already matched the target resize dimensions, so @stevie-mayhew introduced the $force_resample property to prevent this scenario.

I thought it made sense to turn it on be default for SS4, to make image output more consistent and protect users from large downloads. Appears I dropped the ball on updating the docs though, sorry!

@dhensby let me know if you'd like me to update the docs since I should have done it in the first place ;)

@dhensby
Copy link
Contributor

dhensby commented Jun 28, 2017

@jonom yes please if you have the time!

@youpixxl
Copy link

youpixxl commented Mar 2, 2018

Hello,
Sorry I know this one is closed by I am still seeing inconsistency between doc and vendor files.
I just run a new composer installation ^4 to check and in there force_resample is still set to false (ImageManipulation.php line 179).

Also the Images documentation page ([https://docs.silverstripe.org/en/4/developer_guides/files/images/]) still mention that force_resample is off by default, and the code given to turn it on did not work for me.

I was finally able to turn it on by changing config on DBFile.php in my config.yml

SilverStripe\Assets\Storage\DBFile:
  force_resample: true

I may have missed something here and if this is the case sorry to reopen this, but as I struggled for sometimes to find this I thought it was worth mentioning it and maybe it could help others.

@tractorcow
Copy link
Contributor

We'll update the docs, thanks for pointing out the inconsistency.

@tractorcow
Copy link
Contributor

Fixed with silverstripe/silverstripe-framework#7908

@chillu
Copy link
Member

chillu commented Mar 5, 2018

@tractorcow Please remember to add story points

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

No branches or pull requests

8 participants