When zooming, the filter region appears to be computed incorrectly for SVG filters applied to HTML content. Steps to reproduce: 1. load attached testcase 2. zoom out (so the text is smaller) 3. zoom in (so the text is bigger than default) Actual results: 2. right and bottom sides of page are cut off 3. extra scrollbars show up Expected results: 2 & 3: no scrollbars and whole page displayed I'm seeing this in pretty current Minefield nightlies on both Windows and Linux.

Seems to be a similar effect to bug 460291

That may well be due to it being an easy mistake to make (making the wrong choice between CSS pixels per appunit vs. device pixels per appunit) than actually being the same bug.

In this case, the problem is probably one or more of the conversions in nsSVGIntegrationUtils.cpp.

There are reftests checked in for this bug in layout/svg/reftests/: svg-effects-area-unzoomed.xhtml svg-effects-area-zoomed-in.xhtml svg-effects-area-zoomed-out.xhtml Prior to the changes in bug 542595, the second fails because of an incorrect scrollbar and the third fails because the area painted is too small. Bug 542595 makes SVG effects no longer contribute to scrollable overflow, and therefore fixes the failure of the second test. But the third still fails, and the underlying bug is still present.

This version uses a red div, without a filter, that should be entirely covered up by the filtered div (but is not covered up when I zoom out).

Basically, overrideBBox needs to be in CSS px, not dev pixels.

Comment on attachment 634564 [details] [diff] [review] patch (In reply to Jonathan Watt [:jwatt] from comment #10) > Basically, overrideBBox needs to be in CSS px, not dev pixels. Ok -- then this comment (in the contextual code) needs an update, then, right? > // overrideBBox is in "user space", in dev pixels: > nsIntRect overrideBBox = > GetPreEffectsVisualOverflowUnion(firstFrame, aFrame, > aPreEffectsOverflowRect, > firstFrameToUserSpace). >- ToOutsidePixels(appUnitsPerDevPixel); >+ ToOutsidePixels(aFrame->PresContext()->AppUnitsPerCSSPixel()); Also: we only use overrideBBox for this call to GetPostFilterBounds: > > nsRect overflowRect = > filterFrame->GetPostFilterBounds(firstFrame, &overrideBBox). >- ToAppUnits(appUnitsPerDevPixel); >+ ToAppUnits(aFrame->PresContext()->AppUnitsPerDevPixel()); ...and the documentation for that method says "The rects are in device pixels" http://mxr.mozilla.org/mozilla-central/source/layout/svg/base/src/nsSVGFilterFrame.h#72 Is that no longer accurate?

(In reply to Daniel Holbert [:dholbert] from comment #11) > Ok -- then this comment (in the contextual code) needs an update, then, > right? Right. Done. > Is that no longer accurate? It never was, that's the problem. SVG bboxes are in user space, aka CSS px, and despite the comment, it was treating the passed in rect as such.

Oh, and I changed the parameters to be gfxRects instead on nsIntRects to avoid being misleading.

Try found some more tests with bogus references that needed to be fixed.

Comment on attachment 634855 [details] [diff] [review] patch Looks good!

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/6c4b38bcdfc7 - the red builds in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&onlyunstarred=1&rev=12f76b9a2d78 say "Looks bad!" ;)

Sorry, I screwed up the resolution of my merge conflict updating to tip before pushing. Should have rebuilt.

The build log is not very clear about what went on, but it looks like the "unused variable" warning is being treated as an error, while only reporting it as a warning. I've fixed that and pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/010c24939cc5 As an aside, it would be really nice if compiler warning that are fatal on tinderbox were also fatal on local developer machines. That unused variable warning is not fatal for me on Mac. Hmm, maybe that's because I'm using clang?

(In reply to Jonathan Watt [:jwatt] from comment #19) > As an aside, it would be really nice if compiler warning that are fatal on > tinderbox were also fatal on local developer machines. That unused variable > warning is not fatal for me on Mac. Hmm, maybe that's because I'm using > clang? Add --enable-warnings-as-errors to your mozconfig.

Sweet, thanks, roc.