-
Notifications
You must be signed in to change notification settings - Fork 32
Conversation
Is this issue being taken into account please ? Thanks |
Yup, @mattheu reproduced, working on fixing the unit tests to track it down. |
Any ETC for this one? |
Hi @jgalea I have pushed the code to fix this to this branch - could you test it and let me know if it's all good? Quick note: the argument name is now |
@jgalea I'll look into example 1, however example 2 is expected. The background fill will not work here, so you will get a small image. I presume you are then forcing the width / height of the image which is upscaling it in the browser? |
@joehoyle That's right, but I think it would be nicer to just give it a black background so at least you get the image size you requested. You can find some examples of the black background fill on http:https://www.wpmayor.com/wordpress-news/ Since we want a particular image size so all images are uniform, having a small image will break that design, and on the other hand browser upscaling makes them look bad, so the black background fill would be the best solution. |
@jgalea I think that's too much of an assumption across the board - for example, what if you have a white site and you want it to fall back to white if it can't auto pad? Perhaps we can have another option for what you are suggesting like:
or something like that. |
@joehoyle Yes that crossed my mind while I was replying, I totally agree with having that fallback option. |
…ther color values needs to become "015"
@jgalea I just pushed a fix, can you give it another go? Thanks! |
@jgalea I pushed a fix for this, can you give it a bash now? Thanks! |
Hi @jgalea Can you attach the original version of the white image you are trying to background fill? The resize issue doesn't seem to be related to this pull-request. As this isn't a support thread - could you open a new issue for that. I read through your notes here, but I am not 100% clear on what you want the last image to look like. Though it looks like maybe your first example (resize + crop) is only doing a crop? Again, without having the actual image + your code example, I am shooting in the dark so you be good to get that on the new ticket too. |
Right, I guess the others are the same then. I thought the class considered the total area rather than just the edges, but it's probably best to extend based on the edges. To eliminate such instances you'd have to make a condition that checks for such thin borders and excludes them when taking the decision of the background fill colour. |
It currently measures the colour of each corner pixel and if each corner is the same colour it fills with that. |
Understood, I imagine you'd like to keep it nice and simple then, rather than complicate it further as I suggested.In that case this issue is probably resolved. I think Joe's idea earlier would also be nice to have implemented: |
Any plans to implement the background fill fallback? |
Merged, created #96 for the fallback color. |
Check the attached example, I uploaded a red box and it filled the background with black.
Example code to see this in evidence:
echo '<img src="' . wpthumb( 'http:https://s8.postimg.org/l0pouom41/image.jpg', 'width=500&height=500&crop=1&background_fill=solid' ) .'"" />