The plan is to add: - a FilterNode interface in Filters.h - two new DrawTarget APIs: CreateFilter and DrawFilter We will use this for SVG and CSS filters.

This is just a dump of what I have locally at the moment. It only compiles on Mac (I think) and needs to be cleaned up.

Comment on attachment 814110 [details] [diff] [review] wip Review of attachment 814110 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/2D.h @@ +11,5 @@ > #include "Rect.h" > #include "Matrix.h" > #include "UserData.h" > > +#include "Filters.h" Can't you forward-declare FilterNode and remove this #include?

roc, I would like to discuss the approach here. I would like to take the API and implement the software fallback via Skia. This is a lot of code that seems hard to get right and performant and it has been implement many times before, including in Skia. I understand there are concerns about skia's performance behavior and pixel scraping, but the software path is a fallback only, the GPU path likely has the same problems, and this is a lot of code and effort to prevent a problem that doesn't seem unique to this code. Can we at least do this incremental? We use skia to land a 100-liner that implements this and we can iterate on an optimized version over time?

At the work week roc, Jeff, Vlad and I agreed on doing it the other way round: Land what I have now, work on the skia implementation directly afterwards, and then upstream everything that our own software implementation does better than Skia into Skia so that we can get rid of our copy. I'll put my updated patches up for review tomorrow. I spent the last week fixing bugs in the Direct2D backend, that's why this bug still looks dormant. (And there was a public holiday in Germany on Friday.) (In reply to Andreas Gal :gal from comment #3) > We use skia to land a 100-liner that > implements this It's going to be more than that - there's some stuff we need that Skia does not implement, and we want to move all filter processing code out to Moz2D so that we can have the nice API where we only pass one filter structure for painting the whole filter instead of calling repeatedly per primitive.

Works for me.

Cool!

FWIW, additional points in favour of taking Markus' code first in addition to comment #4: -- His code has been green on try (at least, it was recently on Mac). We don't know that about the Skia filter code. -- Making the Skia filter code timing-safe is not easy. For one thing, it uses SkScalar all over the place, which is float the way we build Skia (and the way everyone else builds Skia, says Jeff). So we'd need to undefine SK_SCALAR_IS_FLOAT or do a large search-and-replace job on the Skia filter code. Neither of those are very appealing especially in the short term. -- Although this is a lot of new code, the filter implementation part rewrites and replaces a lot of our existing code (currently under SVG), and much of the glue code will be needed even if we use Skia. So it's not as big an increase in complexity as it looks. > the GPU path likely has the same problems, and this is a lot of code and effort to prevent a problem that doesn't > seem unique to this code. Timing attacks are limited to places where we touch the pixel data of rendered Web content with non-constant-time operations. We don't compute over individual Web content pixels in very many places in our code. I don't know how the GPU path is going to work out w.r.t. timing attacks. It may turn out that we face an irreconcilable tradeoff between filter performance and information leakage, and we may decide to resolve that in favour of filter performance. Even if so, I think it's still valuable to have a timing-safe software path that we know we can fall back to if we have to. We never know when something like this will blow up into a big issue. (CSS history sniffing is an example of an information leak we knew about and sat on for years before it suddenly become a big deal and every browser had to fix it (at a small performance and compatibility cost).) Anyway, thanks for comment #5 and I'm looking forward to seeing patches for review here :-).

Looks like this still isn't finished yet, but let's start the review process anyway. There are some remaining issues with the D2D1 backend so I disabled it with some "#if 0"s for now so that we can land something. They are: -- Tiled turbulence sometimes has gaps. I thought this was due to the crop filter in between, but the gaps were still there when I removed all crop filters. -- Component transfer filters do something strange with premultiplication. The docs say that the input can be either premultiplied or unpremultiplied, and that the output is always premultiplied. However, I have no idea how the filter knows whether the input is premultiplied or unpremultiplied, and simply piping the result through an unpremultiply filter unconditionally did not work in some cases. Needs more investigation. Other issues with this patch: -- Currently does not compile on linux on Try due to warnings-as-errors because the SetAttribute methods in FilterNodeSoftware.h cause "overloaded-virtual" warnings. I guess we need tons of "using" declarations? -- It failed to compile on Windows on Try without giving a reason in the error message. Bas, can you help me fix this? https://tbpl.mozilla.org/php/getParsedLog.php?id=30079494&tree=Try&full=1 -- I did not include the DrawTargetSkia and DrawTargetRecording changes in this patch. Other stuff that needs fixing but can probably happen after the initial landing: -- Lighting filters need to use fixed point arithmetic throughout. At the moment it's an unfinished mixture. -- FilterProcessing files need comments about why this setup with the multiple source files was necessary. Basically, some compilers need a special flag when compiling with SSE2 instructions, but that flag can make them produce SSE2 instructions for some scalar code, too, so the flag must not be used for compiling the code that determines whether the user's CPU supports SSE2. Same for NEON. -- There's more but I don't have a complete list right now.

> -- Component transfer filters do something strange with premultiplication. > The docs say that the input can be either premultiplied or unpremultiplied, > and that the output is always premultiplied. However, I have no idea how the > filter knows whether the input is premultiplied or unpremultiplied, and > simply piping the result through an unpremultiply filter unconditionally did > not work in some cases. Needs more investigation. We keep track of whether the surface we're working with contains unpremultiplied or premultipled data in the current SVG implementation. Each surface has a colour model see http://mxr.mozilla.org/mozilla-central/source/layout/svg/nsSVGFilterInstance.cpp#480, http://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/nsSVGFilters.h#110 and http://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/SVGFEDisplacementMapElement.h#72 I think we have reftests for this: feDisplacementMap-alpha-01.svg and feDisplacementMap-colour-01-ref.svg from bug 584322 and bug 603584

Thanks Robert, but I was actually referring to the Direct2D filters, e.g. http://msdn.microsoft.com/en-us/library/windows/desktop/hh706362%28v=vs.85%29.aspx In bug 924103 I'm changing most of the code you referenced (mostly by moving it into FilterSupport).

This fixes some bugs in the lighting normal calculation.

This class implements the algorithm described at https://dvcs.w3.org/hg/FXTF/raw-file/default/filters/index.html#feTurbulenceElement in a faster and hopefully more readable way. I put it into its own class because it's rather elaborate.

The blur filter will make use of AlphaBoxBlur, and it needs to support different blur radii for x and y.

This is all for now. I'll attach the DrawTargetSkia and DrawTargetRecording implementations when I've extracted them.

Comment on attachment 829251 [details] [diff] [review] part 3, v1: Add SIMD.h with scalar + SSE2 implementations Review of attachment 829251 [details] [diff] [review]: ----------------------------------------------------------------- I checked this for structure, not correctness. Looks good to me though. Eventually we'll probably want to split this up a little more somehow, but for now it's fine since it's nice and well-contained chaos.

Comment on attachment 829251 [details] [diff] [review] part 3, v1: Add SIMD.h with scalar + SSE2 implementations Review of attachment 829251 [details] [diff] [review]: ----------------------------------------------------------------- If derf has some time to glance over this it would be of some value I feel. He knows more about this sort of thing than I do.

Comment on attachment 829250 [details] [diff] [review] part 2, v1: API Review of attachment 829250 [details] [diff] [review]: ----------------------------------------------------------------- I wrote and designed a large part of this, I probably shouldn't be reviewing this alone. Matt, please sanity check :)

Comment on attachment 829258 [details] [diff] [review] part 4, v1: Add filter processing SIMD implementations + a few scalar implementations Review of attachment 829258 [details] [diff] [review]: ----------------------------------------------------------------- The general idea looks great! I haven't verified in detail every single algorithm in here, I assume we've used tests to verify its correctness :-). I feel this could use a little more documentations, the couple of functions I looked at in detail it took me quite some time to figure out what they were supposed to do. Just to be sure please use MOZ_ALWAYS_INLINE for the things where inlining is very important. Inline is ignored fairly often, at least in visual studio with our default build flags on windows. ::: gfx/2d/FilterProcessingSIMD-inl.h @@ +303,5 @@ > + i16x8_t s_bbbbgggg1234, s_rrrraaaa1234; > + i16x8_t d_bbbbgggg1234, d_rrrraaaa1234; > + UnpackAndShuffleComponents(s1234, s_bbbbgggg1234, s_rrrraaaa1234); > + UnpackAndShuffleComponents(d1234, d_bbbbgggg1234, d_rrrraaaa1234); > + // Shuffle for double high These sort of comments could be a little more descriptive :). @@ +607,5 @@ > +} > + > +template<typename i32x4_t, typename i16x8_t, uint32_t aCompositeOperator> > +static inline i16x8_t > +CompositeTwoPixels(i16x8_t source, i16x8_t sourceAlpha, i16x8_t dest, const i16x8_t& destAlpha) Did you verify all our compilers inline this nicely and actually remove this switch? Otherwise you'll need to use template specializations without the switch :).

Comment on attachment 829259 [details] [diff] [review] part 5, v1: Add SVGTurbulenceRenderer Review of attachment 829259 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/SVGTurbulenceRenderer.cpp @@ +52,5 @@ > + > +} // unnamed namespace > + > +template<TurbulenceType Type, bool Stitch, typename T> > +SVGTurbulenceRenderer<Type,Stitch,T>::SVGTurbulenceRenderer(const Size &aBaseFrequency, int32_t aSeed, I feel this could probably get a lot of perf improvements from some SSE2 love. But I can imagine perf might not be that important here. ::: gfx/2d/SVGTurbulenceRenderer.h @@ +13,5 @@ > +namespace gfx { > + > +// TODO: Add floatx4 functions to SIMD.h and use them instead of this vec4. > +template<typename T> > +struct vec4 Our stuctnames are capitalized everywhere else, they probably shouold be here as well.

Comment on attachment 829260 [details] [diff] [review] part 6, v1: Make AlphaBoxBlur accept separate X and Y blur radii Review of attachment 829260 [details] [diff] [review]: ----------------------------------------------------------------- Well that was easy ...

Comment on attachment 829261 [details] [diff] [review] part 7, v1: Add FilterNodeSoftware implementation Review of attachment 829261 [details] [diff] [review]: ----------------------------------------------------------------- This patch is going to take me a little bit more time to review, here's some initial comments. ::: gfx/2d/FilterNodeSoftware.cpp @@ +177,5 @@ > + > +// Fast approximate division by 255. It has the property that > +// for all 0 <= n <= 255*255, FAST_DIVIDE_BY_255(n) == n/255. > +// But it only uses two adds and two shifts instead of an > +// integer division (which is expensive on many processors). I saw these classes in other places as well, please ensure we share all of these. You also probably want to MOZ_ALWAYS_INLINE this. @@ +253,5 @@ > + destData += DataOffset(aDest, aDestPoint); > + > + if (BytesPerPixel(aSrc->GetFormat()) == 4) { > + for (int32_t y = 0; y < aSrcRect.height; y++) { > + for (int32_t x = 0; x < aSrcRect.width; x++) { memcpy is significantly faster than this in my tests. @@ +290,5 @@ > + if (BytesPerPixel(aSurface->GetFormat()) == 4) { > + uint32_t sourcePixel = *(uint32_t*)sourcePixelData; > + for (int32_t y = 0; y < aFillRect.height; y++) { > + for (int32_t x = 0; x < aFillRect.width; x++) { > + *((uint32_t*)data + x) = sourcePixel; Do one row, and memcpy it into the rest :). That'll be a lot faster. @@ +317,5 @@ > + data += DataOffset(aSurface, aFillRect.TopLeft()); > + if (BytesPerPixel(aSurface->GetFormat()) == 4) { > + for (int32_t y = 0; y < aFillRect.height; y++) { > + for (int32_t x = 0; x < aFillRect.width; x++) { > + *((uint32_t*)data + x) = *((uint32_t*)sampleData + x); memcpy!

Clang turned these loops into memcpy for me, I assumed other compilers would do the same ;)

This adds some Float32 APIs for SIMD turbulence rendering.

This uses the float SIMD functions for turbulence rendering for a 2.5x speedup. I've moved the implementation into an -inl.h header file because it needs to be included in FilterProcessingScalar.cpp and FilterProcessingSSE2.cpp which are compiled with different compiler flags.

I've tweaked some algorithms and added SIMD code for the arithmetic combine filter. I've also added some comments.

I applied the PodCopy / PodZero improvements you suggested.

Most of this code was written by you. Who should review that part? There were some problems with the original code, here's how I fixed them: -- If you had a SourceSurface that was the snapshot of a DrawTargetD2D, and the SourceSurface became detached because the DrawTarget went away, calling FilterNodeD2D::SetInput with that SourceSurface did not work because you couldn't get an ID2D1Image for it. So I made FilterNodeD2D take a reference to the DrawTarget it was created for, so that it can use that DrawTarget to get the right ID2D1Image for those SourceSurfaces. I made GetImageForSurface public on DrawTargetD2D1. -- The convolve matrix filter had no way to respect the source rect attribute (which I had to add for SVG compatibility). I had to resort to some tricks for that, details are in the comments. -- Component transfer filters premultiply in a way that doesn't really fit our use case. See the comments in the code for more details. -- I moved FilterNode creation code into a static method of FilterNodeD2D1 so that it can be shared between DrawTargetD2D and DrawTargetD2D1. I also added handling for some lighting enums, fixed up turbulence rendering by adding a rect attribute, and added the alpha mode attribute for the color matrix filter.

Comment on attachment 8334059 [details] [diff] [review] part 9, v1: Add Direct2D 1.1 implementation Review of attachment 8334059 [details] [diff] [review]: ----------------------------------------------------------------- Jeff, can you get to this before you go on PTO?

Comment on attachment 8334047 [details] [diff] [review] part 7, v2: Add FilterNodeSoftware implementation Review of attachment 8334047 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/FilterNodeSoftware.cpp @@ +289,5 @@ > + *((uint32_t*)data + x) = sourcePixel; > + } > + } else if (BytesPerPixel(aSurface->GetFormat()) == 1) { > + uint8_t sourcePixel = *sourcePixelData; > + for (int32_t x = 0; x < aFillRect.width; x++) { memset @@ +344,5 @@ > + } else if (BytesPerPixel(aSurface->GetFormat()) == 1) { > + for (int32_t y = 0; y < aFillRect.height; y++) { > + int8_t sampleColor = *sampleData; > + for (int32_t x = 0; x < aFillRect.width; x++) { > + data[x] = sampleColor; memset

(In reply to Bas Schouten (:bas.schouten) from comment #36) > Comment on attachment 8334047 [details] [diff] [review] > part 7, v2: Add FilterNodeSoftware implementation > > Review of attachment 8334047 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/2d/FilterNodeSoftware.cpp > @@ +289,5 @@ > > + *((uint32_t*)data + x) = sourcePixel; > > + } > > + } else if (BytesPerPixel(aSurface->GetFormat()) == 1) { > > + uint8_t sourcePixel = *sourcePixelData; > > + for (int32_t x = 0; x < aFillRect.width; x++) { > > memset > > @@ +344,5 @@ > > + } else if (BytesPerPixel(aSurface->GetFormat()) == 1) { > > + for (int32_t y = 0; y < aFillRect.height; y++) { > > + int8_t sampleColor = *sampleData; > > + for (int32_t x = 0; x < aFillRect.width; x++) { > > + data[x] = sampleColor; > > memset How? I think I'd need something like memset_pattern4 for this.

This fixes the blur source inflation size calculation to match up with the one in Blur.cpp.

A transform filter simplifies the implementation of the <feImage> SVG filter.

Comment on attachment 8334531 [details] [diff] [review] part 10, v1: Add transform filter and remove offset filter Review of attachment 8334531 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/FilterNodeSoftware.cpp @@ +1017,5 @@ > + dt->SetTransform(transform); > + dt->DrawSurface(input, r, r, DrawSurfaceOptions(mFilter)); > + > + RefPtr<SourceSurface> result = dt->Snapshot(); > + RefPtr<DataSourceSurface> resultData = result->GetDataSurface(); This is very bad for D2D. (you'll trigger readback) I suspect we'll want to pick one backend that we know is fast with software and do CreateDrawTarget with that backend. Presumably Skia or Cairo. This also prevents the ugliness of passing the type around.

Does the Moz2D standalone repo build system currently require at least Skia or Cairo to be enabled? If not, what should I do about that? I agree that doing it this way with D2D is bad in the general case, but it's not as bad when the transform filter is the first in the chain. When we use software filters for D2D source surfaces, we need to do readback at some point anyway, and doing it after the transform will be faster because then we won't need to do the transform in software. Transforms that are simple translations don't hit the readback path because they take the early exit with > if (transform.IsIdentity() && srcRect.Size() == aRect.Size()) { > return input; > }

... and the patches in bug 924103 will only use non-offset transform filters that apply directly to input source surfaces, and not in the middle of a filter chain.

Comment on attachment 8334059 [details] [diff] [review] part 9, v1: Add Direct2D 1.1 implementation Review of attachment 8334059 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/FilterNodeD2D1.cpp @@ +179,5 @@ > +void ConvertValue(uint32_t aType, uint32_t aAttribute, IntSize &aValue) > +{ > + switch (aType) { > + case FILTER_MORPHOLOGY: > + if (aAttribute == ATT_MORPHOLOGY_RADII) { Can you add a comment about what this is for?

This uses BACKEND_CAIRO always. DrawTargetSkia::DrawSurface currently does not support drawing pure DataSourceSurfaces.

On the D2D1 transform filter we need to set border mode to "hard", because an <feImage> with an image that's just a single opaque pixel is expected to stay opaque when upscaled - it shouldn't have transparent fringes. This is tested by the reftest anim-feImage-preserveAspectRatio-01.svg.

(In reply to Markus Stange [:mstange] from comment #37) > (In reply to Bas Schouten (:bas.schouten) from comment #36) > > Comment on attachment 8334047 [details] [diff] [review] > > part 7, v2: Add FilterNodeSoftware implementation > > > > Review of attachment 8334047 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > ::: gfx/2d/FilterNodeSoftware.cpp > > @@ +289,5 @@ > > > + *((uint32_t*)data + x) = sourcePixel; > > > + } > > > + } else if (BytesPerPixel(aSurface->GetFormat()) == 1) { > > > + uint8_t sourcePixel = *sourcePixelData; > > > + for (int32_t x = 0; x < aFillRect.width; x++) { > > > > memset > > > > @@ +344,5 @@ > > > + } else if (BytesPerPixel(aSurface->GetFormat()) == 1) { > > > + for (int32_t y = 0; y < aFillRect.height; y++) { > > > + int8_t sampleColor = *sampleData; > > > + for (int32_t x = 0; x < aFillRect.width; x++) { > > > + data[x] = sampleColor; > > > > memset > > How? I think I'd need something like memset_pattern4 for this. Isn't byte being write to data? memset(data, sampleColor, aFillRect.width), no?

You're right! Somehow I missed that.

I'll deal with these after the initial landing. At the moment RecordedEvent.cpp isn't completely in sync with moz2d (it's missing http://hg.mozilla.org/users/bschouten_mozilla.com/moz2d/rev/5fe087148226 and http://hg.mozilla.org/users/bschouten_mozilla.com/moz2d/rev/ae3b33c3f929 ) and DrawTargetSkia can't handle non-Skia SourceSurfaces in DrawSurface.

Comment on attachment 8336076 [details] [diff] [review] part 11, v1: Stub out filter APIs on DrawTargetRecording and DrawTargetSkia Review of attachment 8336076 [details] [diff] [review]: ----------------------------------------------------------------- I'd prefer if we just uploaded the code needed from Moz2D and skip this step.

Most of this code is from you, but I converted a few more places to use GetBitmapForSurface and I added a MOZ_CRASH in the non-SURFACE_DATA case so that we don't silently ignore incompatible surfaces. This behavior is consistent with the CG in the patch for bug 930956.

Bas wrote this patch and I reviewed it.

Bas wrote this patch and I reviewed it. I made a small change: I renamed the TYPE_* enum values to ARGTYPE_* because some Mac system header (/usr/include/ConditionalMacros.h) does #define TYPE_BOOL 1. The compiler didn't tell me how that header was included and I didn't bother to find out.

Comment on attachment 8336739 [details] [diff] [review] part 12, v1: Make sure we're only calling DrawSurface with SURFACE_DATA surfaces Review of attachment 8336739 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/FilterNodeSoftware.cpp @@ +594,5 @@ > printf("</pre>\n"); > #endif > > + RefPtr<DataSourceSurface> dataSurface = result; > + if (dataSurface->GetType() != SURFACE_DATA) { I'm confused, by this, GetDataSurface() on DataSourceSurface will simply return 'this'. If we're able to access Stride and Data on a surface, the type should always be SURFACE_DATA. But regardless, our drawing backends should simply be doing a GetDataSurface() on whatever they don't know how to draw. (This could be slow, but that's a risk for the caller) If they're not doing that that's a bug we should probably fix.

Comment on attachment 8336739 [details] [diff] [review] part 12, v1: Make sure we're only calling DrawSurface with SURFACE_DATA surfaces With the patches in bug 943614 this part is no longer needed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5bc7ed8af53 https://hg.mozilla.org/integration/mozilla-inbound/rev/6e9afaacf242 https://hg.mozilla.org/integration/mozilla-inbound/rev/c57ffc30cd43 https://hg.mozilla.org/integration/mozilla-inbound/rev/d1f39d041ccb https://hg.mozilla.org/integration/mozilla-inbound/rev/5a32026fcfcf https://hg.mozilla.org/integration/mozilla-inbound/rev/7f470ad20b6b https://hg.mozilla.org/integration/mozilla-inbound/rev/8d737d52c4cb https://hg.mozilla.org/integration/mozilla-inbound/rev/272bb40f463f https://hg.mozilla.org/integration/mozilla-inbound/rev/9fe4a520bf31 https://hg.mozilla.org/integration/mozilla-inbound/rev/172e1c46f2d6 https://hg.mozilla.org/integration/mozilla-inbound/rev/63c02ba3a94e https://hg.mozilla.org/integration/mozilla-inbound/rev/e59bb20fc8dc https://hg.mozilla.org/integration/mozilla-inbound/rev/77ba34a72f88

https://hg.mozilla.org/mozilla-central/rev/d5bc7ed8af53 https://hg.mozilla.org/mozilla-central/rev/6e9afaacf242 https://hg.mozilla.org/mozilla-central/rev/c57ffc30cd43 https://hg.mozilla.org/mozilla-central/rev/d1f39d041ccb https://hg.mozilla.org/mozilla-central/rev/5a32026fcfcf https://hg.mozilla.org/mozilla-central/rev/7f470ad20b6b https://hg.mozilla.org/mozilla-central/rev/8d737d52c4cb https://hg.mozilla.org/mozilla-central/rev/272bb40f463f https://hg.mozilla.org/mozilla-central/rev/9fe4a520bf31 https://hg.mozilla.org/mozilla-central/rev/172e1c46f2d6 https://hg.mozilla.org/mozilla-central/rev/63c02ba3a94e https://hg.mozilla.org/mozilla-central/rev/e59bb20fc8dc https://hg.mozilla.org/mozilla-central/rev/77ba34a72f88

Patch part 11 has a side-effect of making skia unusable on Linux because DrawSurface is called frequently with a cairo surface (from gfxXlibNativeRenderer.cpp and friends) and the patch introduces a MOZ_CRASH for that condition. The fact that these parts still use cairo seems to be why the chrome (tabs, etc.) look so bad on skia on linux at the moment. I tried a quick hack of converting the cairo surfaces to DataSourceSurface in DrawTargetSkia and that fixes the crash and also makes the chrome look good. Obviously though this is not the optimal fix. Should we however introduce this as a temporary work around until the hardcoded cairo parts can be removed from thebes for gtk/x11?

Sounds good to me. Can you please open a new bug for this problem?

Done: https://bugzilla.mozilla.org/show_bug.cgi?id=945314

Hi, I noticed an uninitialized value usage when I ran thunderbird (comm-central) under valgrind. The reported backtrace suggests that it is caused by function DoUnpremultipcationCalculation_SSE2 (and eventually the inlined DoUnpremultiplicationCalculation_SIMD ) introduced by patch(es) in this bugzilla entry. I wonder if someone can take a look at Bug 964827 - Use of uninitialized value in 2D filtering code. TIA

De-prioritizing QA verification of this bug. Please remove the [qa-] whiteboard tag and add the verifyme keyword if you think this needs our attention before Firefox 28 is released.