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

Support for retina (high dpi) images using Imagemagick #50

Closed
rocxa opened this issue Oct 10, 2016 · 19 comments · Fixed by #371
Closed

Support for retina (high dpi) images using Imagemagick #50

rocxa opened this issue Oct 10, 2016 · 19 comments · Fixed by #371

Comments

@rocxa
Copy link
Contributor

rocxa commented Oct 10, 2016

Imagemagick has the ability to resample images to a higher dpi. This could be useful for retina images in emails to support high definition devices. Im not sure how to do it in mosaico yet though.
http://php.net/manual/en/imagick.resampleimage.php

@rocxa rocxa changed the title Support for retina (high dpi) images using imagemajick Support for retina (high dpi) images using Imagemagick Oct 10, 2016
@magnolia61
Copy link

magnolia61 commented Jan 23, 2017

This is a much needed improvement for this extension for proper display on regular smartphones.
See attached screenshot of the display on my phone. Exactly the same picture is uploaded. The upper image is uploaded in a 534x200 block. The lower picture in a 166x90 block.
Because the template is responsive, on a smartphone both pictures are show both in the same width which means the pictures from the 166x90 block is stretched and is show in a very bad resolution.
screenshot_20170123-114605

deepak-srivastava added a commit that referenced this issue Mar 1, 2017
totten added a commit to totten/uk.co.vedaconsulting.mosaico that referenced this issue Mar 23, 2017
@Stoob
Copy link
Contributor

Stoob commented Jun 14, 2018

@deepak-srivastava I'm seeing this with version 2.0-beta3.1512698300 although I'm not convinced 'retina' is the problem. Instead it looks as though the image is being resized twice: downsized first and then up-sized again...resulting in a blurry appearance on any display
I have a client who could offer some sponsorship. Any thoughts?

@themak1985
Copy link

I am bumping this issue up. I am not sure what Mosaico is doing but our images look soft on desktop but really horrible on mobile. Is imagemagick doing more work than it should?

@kelizoliva
Copy link

Retina or not, I've observed for my clients that generally the image quality with the mosaic templates is quit poor.

An easy solution to this would be to have Imagick double the size of the images. So for example the 534x200 image could instead have an actual size of 1068x400 rendered by Imagick, and then the styling of the image in the template could resize the image to 524x200. This could probably be accomplished with a custom built template.

I would advocate that the default template be adjusted to provide higher image quality, so that non-technical users of the extension are not stuck with poor quality images in their email blasts.

@themak1985
Copy link

Has anyone found a solution? We are considering going back to mailchimp - the images all look bad in mosaico - really bad when viewing on mobile.

@Stoob
Copy link
Contributor

Stoob commented Feb 17, 2020

@veda-consulting Are you and your team interested in providing an estimate to fix, and we could combine funds to sponsor this improvement?

@francescbassas
Copy link
Contributor

francescbassas commented Feb 18, 2020

Maybe the issue should be fixed and discussed on the Mosaico side. See voidlabs/mosaico#531. Even maybe in the versafix-template repository https://github.com/voidlabs/versafix-template.

@themak1985
Copy link

themak1985 commented Feb 18, 2020

Here is also something interesting. Images used in templates are treated differently than in production emails. Attached you will see two of the same images (same source file), both on the triple block with text. The top image is what we had in the template, the bottom image, we added in the production email.

Edit: Actually - this is a fluke - I have not been able to replicate.

Screenshot_20200218-131359

@deepak-srivastava
Copy link
Collaborator

There are a couple of workarounds possible while voidlabs/mosaico#531 tries to come up with providing better control for delegating the resize decisions to template and user.

One gross hack / solution (for a best quality) is to don't resize at all. Try this experimental patch - mountev@9009f3e
In most cases html will do a good job of displaying resized images.
Problem here is most dummy user would upload too many high resolution images which is probably not very good for an email, as it may slow down the rendering of email.
User can make best use of such methods if they able to make decisions on their own.

Other intermediate solution (for a medium quality) could be to double the width and height of resize being requested. Which would still solve the problem but you may not see a best quality image.

If agreed by community, these approach could be exposed as a mosaico config for users to decide.

Here is one result of experimental patch :
image

@themak1985
Copy link

@deepak-srivastava interesting workaround - essentially your patch would require folks optimize images on their own which for us is fine as we typically web optimize on our own anyways.

The compromise of doubling the width and height of resize being requested is a good idea. Though it might make more sense to triple or quadruple the width and height of the resize being requested. Having the option in mosaico config for users to decide is great.

Thank you for your attention and work on this.

@Stoob
Copy link
Contributor

Stoob commented Feb 19, 2020

I kind of like the 2x (or 3x) resize solution as it provides for an acceptable (although not ideal) outcome without refactoring too much code. Still I would like to see a proper solution and will ask if my clients can contribute. Is anyone else here willing to contribute $/£/€ to the effort?

@deepak-srivastava
Copy link
Collaborator

I agree that resizing by a factor (2x / 3x) is better approach.

A solution with config parameter for accepting resize multiplication factor, as long as the result is within the limits of original dimensions of image.
This may require different levels of config and factor though - for e.g

  • for cover images (2 or 3 column layout) where width is between 166 and 258 px - multiply by 3x.
  • for resize (1 column / big images) where width is between 534 and 570 px - multiply by 2x.
  • another level?

The estimate for the work is $1120 to $1440.

I can try and get a "Make it Happen" setup and proceed once minimum margin is paid. -OR-
Wait for someone who has time and energy to implement and submit the solution with the given spec.

@kelizoliva
Copy link

I have a client who may be interested in contributing a little toward this if there is something set up for them to contribute to.

@deepak-srivastava
Copy link
Collaborator

For those looking to fund this piece, here is the MIH - https://civicrm.org/make-it-happen/mosaico-support-high-resolution-images

@jamienovick
Copy link

@deepak-srivastava do you have any more details on how the configuration would work? Would this be on a "per image" basis, "per email" or "global" setting?

If it is per image then would it be a modification to the versafix template or some other method?

Sorry for all the questions but sounds positive!

Best

J

@deepak-srivastava
Copy link
Collaborator

@jamienovick It's a global setting. The proposed solution is based on the idea that when uploaded image is high resolution, the mosaico editor (irrespective of template being used) doesn't trim it down to very required size (in pixels), but instead keep some buffer (2x or 3x) so upscale doesn't look distorted or low resolution.

@kelizoliva
Copy link

Palante just contributed to the Make it Happen, and I urged one of our clients to contribute as well (not sure whether they will, but hopefully!).

@deepak-srivastava
Copy link
Collaborator

Here is the PR #371 in case someone is willing to test.

@Stoob
Copy link
Contributor

Stoob commented Apr 16, 2020

I have tested this PR and it works as intended. Thanks @deepak-srivastava

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

Successfully merging a pull request may close this issue.

8 participants