These eventually re-surface at mozilla::image::imgFrame::Optimize() as (presumably) the generated image(s) are checked to see if they are all one colour.

Generated on x86_64-linux, when loading http://tangerine-tycoon.wikia.com/wiki/Tangerine_Tycoon_Wiki

A bit of digging shows this happens when the filter_length is not divisible by 4, that is, this bit int r = filter_length & 3; if (r) { memcpy(&buffer, row_to_filter, r * 4); Problem is that |buffer| contains 4 elements, all of which make it to imgFrame::Optimize() (presumably), but the memcpy doesn't initialise all of them.

This zeroes out the otherwise-uninitialised 1, 2 or 3 vector elements in |buffer|. It looks as if ConvolveVertically_SSE2_impl, further down, has the same problem, so this patch fixes that too.

So this was introduced by bug 1054510 making changes to the upstream code that I wasn't aware of. I'm going to investigate whether we should just do what upstream is doing instead.

Jeff, is there any progress on this? Anything I can do to help move it along?

I tried switching to something closer to upstream and ran into some address sanitizer problems. I've attached the WIP patch

It looks like there might be a bug in the upstream that's causing this. I'll continue to investigate.

Giving the convolver some extra room helps things.

It's still broken: https://treeherder.mozilla.org/#/jobs?repo=try&revision=daf9a8f6b357

(In reply to Jeff Muizelaar [:jrmuizel] from comment #9) > It's still broken: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=daf9a8f6b357 Wrong patch. https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d21c9de8f24 will hopefully work better.

This worked better: https://treeherder.mozilla.org/#/jobs?repo=try&revision=61c7a54aaca6

This adds 15 bytes of padding to the rows so that we don't have to worry about reading past the bounds and allows us to use a version of the convolver that's much closer to upstream. Seth you're welcome to review the whole change but I'm mostly interested in the Downscaler.cpp changes.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #12) > Created attachment 8650588 [details] [diff] [review] > Make convolver more like upstream From my point of view, looks good. I can no longer reproduce the V complaint with this in place.

Comment on attachment 8650588 [details] [diff] [review] Make convolver more like upstream Review of attachment 8650588 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. (I didn't look closely at the convolver code changes; presumably taking us closer to upstream is also taking us in the direction of correctness.) It'd be good to test locally on a page with scaled JPEGs (imgur.com is good for this), just to sanity check that there's no obvious bustage. For now we only have a very small number of tests that run with HQ scaling enabled, and when HQ scaling is disabled we also don't use DDD. I'd like to eventually turn on DDD for all tests, but additional work is needed to avoid the kinds of races that HQ scaling is subject to.