Bounds Calculation for Text stroke with opacity < 1 not correct

• Component: SVG
• Status: RESOLVED FIXED

[Jeff Schiller]

The attached test case demonstrates an error in Webkit:  When a <text> element has an opacity < 1.0, the calculation for the bounds of the stroke are incorrect (the left and right edges are erroneously clipped)

[Jeff Schiller]

Created attachment 29796 [details]
Test Case showing the problem

[Dirk Schulze]

I see this problem on webkitgtk too.

[Eric Seidel (no email)]

The problem here would be that FloatRect RenderSVGText::repaintRectInLocalCoordinates() const isn't including the stroke correctly, I suspect.  It has some code which tries to include the stroke, but it must be wrong.

It's also possible that this is a CG bug... because I don't think we specify the size of the transparency layer when we tell CG that we need to render transparent text... hum.

[Eric Seidel (no email)]

krit and jamesr point out that the G in the second bottom left Gradient is cut off at the top in:
http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-pservers-grad-08-b.html

Probably the same (or at least related) problem.

[Dirk Schulze]

(In reply to comment #4)
> krit and jamesr point out that the G in the second bottom left Gradient is cut
> off at the top in:
> http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-pservers-grad-08-b.html
> 
> Probably the same (or at least related) problem.

It's not direclt related to this. The G-bug was a problem in SVGResourceGradient and it's neither a CG-bug. I see it on WebKitGtk. We must have a check, if opacity<1 use objectBoundingBox or something like that.

[Eric Seidel (no email)]

There are a couple places where we're still using objectBoundingBox() where we mean repaintRectInLocalCoordinates()  However changing those didn't fix the entire problem for full-pservers...

I think the full-pservers... bug is a separate issue.  This bug is likely cause by this code:

void SVGRootInlineBox::paint(RenderObject::PaintInfo& paintInfo, int tx, int ty)
{
    if (paintInfo.context->paintingDisabled() || paintInfo.phase != PaintPhaseForeground)
        return;

    RenderObject::PaintInfo savedInfo(paintInfo);
    paintInfo.context->save();

    SVGResourceFilter* filter = 0;
    FloatRect boundingBox(tx + x(), ty + y(), width(), height());

    // Initialize text rendering
    prepareToRenderSVGContent(renderer(), paintInfo, boundingBox, filter);

Which is using a bad boundingBox calculation there.  prepareToRenderSVGContent needs to be changed to take a separate objectBoundingBox() and repaintRectInLocalCoordinates() and use the repaint rect for  the opacity clip (which we do set up manually) and the objectBoundingBox for filter setup (which we're not actually doing in ToT since filters are turned off).

maybe x(), y(), width(), height() is right, but I doubt it includes stroke width.  SVGRootInlineBox probably needs its own repaintRectInLocalCoordinates() method.

[Eric Seidel (no email)]

I've split the 'G' painting bug off into its own bug: https://bugs.webkit.org/show_bug.cgi?id=25460

[Dirk Schulze]

(In reply to comment #6)
> There are a couple places where we're still using objectBoundingBox() where we
> mean repaintRectInLocalCoordinates()  However changing those didn't fix the
> entire problem for full-pservers...
> 
> I think the full-pservers... bug is a separate issue.  This bug is likely cause
> by this code:
> 
> void SVGRootInlineBox::paint(RenderObject::PaintInfo& paintInfo, int tx, int
> ty)
> {
>     if (paintInfo.context->paintingDisabled() || paintInfo.phase !=
> PaintPhaseForeground)
>         return;
> 
>     RenderObject::PaintInfo savedInfo(paintInfo);
>     paintInfo.context->save();
> 
>     SVGResourceFilter* filter = 0;
>     FloatRect boundingBox(tx + x(), ty + y(), width(), height());
> 
>     // Initialize text rendering
>     prepareToRenderSVGContent(renderer(), paintInfo, boundingBox, filter);
> 
> Which is using a bad boundingBox calculation there.  prepareToRenderSVGContent
> needs to be changed to take a separate objectBoundingBox() and
> repaintRectInLocalCoordinates() and use the repaint rect for  the opacity clip
> (which we do set up manually) and the objectBoundingBox for filter setup (which
> we're not actually doing in ToT since filters are turned off).
> 
> maybe x(), y(), width(), height() is right, but I doubt it includes stroke
> width.  SVGRootInlineBox probably needs its own repaintRectInLocalCoordinates()
> method.

I thing you're right. We give the objectBoundingBox to prepareToRenderSVGContent instead of the repaint Rect. This is wrong. It's not neccessary to give the obb to prepareToRenderSVGContent. It already has the object and can ask for the obb itself.

[Dirk Schulze]

This is fixed with trunk. Closing the bug.