Background fill not working.

[jgalea]

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://s8.postimg.org/l0pouom41/image.jpg', 'width=500&height=500&crop=1&background_fill=solid' ) .'"" />

[mecha]

Is this issue being taken into account please ? Thanks

[joehoyle]

Yup, @mattheu reproduced, working on fixing the unit tests to track it down.

[jgalea]

Any ETC for this one?

[joehoyle]

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 background_fill=auto

[jgalea]

It's still not working Joe, see the examples attached.

Example 1 - Source Example 1 - Result

Example 2 - Source Example 2 - Result

In the first example you'll see that an incorrect background colour was applied, while in the second example the image is being enlarged and looking bad.

[joehoyle]

@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?

[jgalea]

@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://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.

[joehoyle]

@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:

array( 'background_fill' => 'auto,0000000000' )

or something like that.

[jgalea]

@joehoyle Yes that crossed my mind while I was replying, I totally agree with having that fallback option.

[joehoyle]

@jgalea I just pushed a fix, can you give it another go? Thanks!

[joehoyle]

@jgalea I pushed a fix for this, can you give it a bash now? Thanks!

[jgalea]

Definitely better now Joe, some of them don't work perfectly though, I noticed it mostly on images with white backgrounds, they get a grey bg, here are some examples:

[joehoyle]

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.

[jgalea]

Here;s the original image Joe, I'll open another ticket for the other issue.

[willmot]

That image has a grey inner-shadow/border. That's the reason it get's a grey background-fill.

[jgalea]

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.

[willmot]

thought the class considered the total area rather than just the edges

It currently measures the colour of each corner pixel and if each corner is the same colour it fills with that.

[jgalea]

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: https://github.com/humanmade/WPThumb/pull/87#issuecomment-28391307

[jgalea]

Any plans to implement the background fill fallback?

[joehoyle]

Merged, created #96 for the fallback color.