[Webkit-unassigned] [Bug 40793] [GTK] The size of the shadow image uses the standard deviation size instead of the blur radius

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=40793


Dirk Schulze <krit at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #60298|review?                     |review-
               Flag|                            |




--- Comment #5 from Dirk Schulze <krit at webkit.org>  2010-07-01 23:33:09 PST ---
(From update of attachment 60298)
WebCore/platform/graphics/cairo/FontCairo.cpp:97
 +          float radius = 0.f;
Please use jut 0 without the .f at the end (according to the change in the style guide)

WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:180
 +      float radius = 0.f;
dito

WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:580
 +      float radius = 0.f;
dito

WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:871
 +      radius = std::min(1000.f, radius);
dito

WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:873
 +      FloatSize resolution = FloatSize(1.0f, 1.0f);
...(1, 1) Also, the resolution doesn't change here, add it to the funtion call directly

WebCore/platform/graphics/cairo/ImageCairo.cpp:143
 +          float radius = 0.f;
no .f

WebCore/platform/graphics/filters/FEGaussianBlur.cpp:99
 +  static void kernelPosition(int boxBlur, unsigned std, int& dLeft, int& dRight)
Either you use inline, or you add the function to the header. I bet that is the reason why Mac-build fails.

WebCore/platform/graphics/filters/FEGaussianBlur.cpp:125
 +  static const float gaussianKernelFactor = (3.f * sqrt(2 * M_PI) / 4.f);
Please add this line right after the includes. Name it gGaussianKernelFactor to make sure that this is a constant. No .f. I think M_PI is wrong here (problems on Mac builds). please use piDouble again.

WebCore/platform/graphics/filters/FEGaussianBlur.cpp:141
 +      unsigned sdx = static_cast<unsigned>(floor(std::max(m_x, 2.f) * filter->filterResolution().width() * gaussianKernelFactor + 0.5f));
no .f

WebCore/platform/graphics/filters/FEGaussianBlur.cpp:142
 +      unsigned sdy = static_cast<unsigned>(floor(std::max(m_y, 2.f) * filter->filterResolution().height() * gaussianKernelFactor + 0.5f));
dito

WebCore/platform/graphics/filters/FEGaussianBlur.cpp:181
 +      sdx = std::max((radius.x() * 2 / 3 - 0.5f) / (resolution.width() * gaussianKernelFactor), 0.f);
Please use: using namespace std; Saw this multiple times before, please fix it there as well.
no .f

WebCore/platform/graphics/filters/FEGaussianBlur.cpp:182
 +      sdy = std::max((radius.y() * 2 / 3 - 0.5f) / (resolution.height() * gaussianKernelFactor), 0.f);
dito

I'm quite sure, that this need updates of pixel tests. Can you run layout tests with a tolerance of zero please? I'm also not sure about kernelPosition. The spec for feGaussianBlur changed recently and a blurring radius of 0 is allowed now. If a stdDeviation for any direction is zero, the image is just not blurred for this direction. At the moment we break and don't draw the blurring at all. Now that you try to fix the SVG filter as well, and I'm realy glad about this :-), can you take about this fact as well? It might also be possible to move the feGaussianBlur fixes to another patch,if you think this is not the right place.

Great patch! Please fix the style problems and think about the last comment about kernelPosition. r- for now, also for breaking Mac.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.