Bug 25404 - Bounds Calculation for Text stroke with opacity < 1 not correct
Summary: Bounds Calculation for Text stroke with opacity < 1 not correct
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 23881
Blocks:
  Show dependency treegraph
 
Reported: 2009-04-25 20:30 PDT by Jeff Schiller
Modified: 2010-07-01 09:11 PDT (History)
1 user (show)

See Also:


Attachments
Test Case showing the problem (380 bytes, image/svg+xml)
2009-04-25 20:30 PDT, Jeff Schiller
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Schiller 2009-04-25 20:30:08 PDT
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)
Comment 1 Jeff Schiller 2009-04-25 20:30:25 PDT
Created attachment 29796 [details]
Test Case showing the problem
Comment 2 Dirk Schulze 2009-04-26 23:11:53 PDT
I see this problem on webkitgtk too.
Comment 3 Eric Seidel (no email) 2009-04-28 15:17:32 PDT
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.
Comment 4 Eric Seidel (no email) 2009-04-28 15:20:47 PDT
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.
Comment 5 Dirk Schulze 2009-04-28 15:45:01 PDT
(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.
Comment 6 Eric Seidel (no email) 2009-04-28 15:58:02 PDT
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.
Comment 7 Eric Seidel (no email) 2009-04-28 16:00:42 PDT
I've split the 'G' painting bug off into its own bug: https://bugs.webkit.org/show_bug.cgi?id=25460
Comment 8 Dirk Schulze 2009-12-30 14:44:54 PST
(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.
Comment 9 Dirk Schulze 2010-07-01 09:11:39 PDT
This is fixed with trunk. Closing the bug.