Bug 76800 - <feImage> has problems referencing local elements
Summary: <feImage> has problems referencing local elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks: 10403
  Show dependency treegraph
 
Reported: 2012-01-22 11:09 PST by Nikolas Zimmermann
Modified: 2012-01-23 04:43 PST (History)
6 users (show)

See Also:


Attachments
Patch (94.70 KB, patch)
2012-01-22 11:49 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch v2 (94.78 KB, patch)
2012-01-22 11:58 PST, Nikolas Zimmermann
zherczeg: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2012-01-22 11:09:49 PST
<svg viewBox="0 0 400 400" width="666" height="666">
<defs>
    <ellipse id="e" cx="210" cy="210" rx="50" ry="50"/>
     <filter id="f">
        <feImage xlink:href="#e"/>
    </filter>
</defs>
<rect width="400" height="400" transform="scale(2)" filter="url(#f)" />
</svg>

Both the viewBox trafo on the outermost <svg>, as well as the transform attribute of the rect are not taken into account when rendering the target <ellipse> into an ImageBuffer.
Currently SVGFEImageElement resolves the local reference, by rendering the target element into an Image, and pass that to the SVGFEImage filter.

In order to properly fix the bugs I mentioned, we need to render the target image right in the SVGFEImage filter effect when we have access to the correct absoluteTransforms/absoluteSubregion, to build up the ImageBuffer in absolute coordinates, as big as the final rendered size on screen, instead of just creating an ImageBuffer with the size of the renderers objectBoundingBox, as SVGFEImageElement is doing right now:

-        IntRect targetRect = enclosingIntRect(renderer->objectBoundingBox());
-
-        if (targetRect.isEmpty())
-            return 0;
-
-        m_targetImage = ImageBuffer::create(targetRect.size(), ColorSpaceLinearRGB);
-
-        AffineTransform contentTransformation;
-        SVGImageBufferTools::renderSubtreeToImageBuffer(m_targetImage.get(), renderer, contentTransformation);

We should also never create a new intermediate ImageBuffer for that, but instead render the referenced target element right into the "Image* resultImage" that each filter effect has available.
Uploading a patch soon. This is one step towards fixing bug 10403.
Comment 1 Nikolas Zimmermann 2012-01-22 11:49:35 PST
Created attachment 123498 [details]
Patch
Comment 2 WebKit Review Bot 2012-01-22 11:52:19 PST
Attachment 123498 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

Last 3072 characters of output:
outTests/platform/chromium/test_expectations.txt:3924:  Path does not exist. svg/filters/feImage-preserveAspectRatio.svg  [test/expectations] [2]
ERROR: FAILURES FOR <lucid, x86, debug, cpu> in /mnt/git/webkit-style-queue/LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:3924 Path does not exist. svg/filters/feImage-preserveAspectRatio.svg
LayoutTests/platform/chromium/test_expectations.txt:3924:  Path does not exist. svg/filters/feImage-preserveAspectRatio.svg  [test/expectations] [2]
ERROR: FAILURES FOR <lucid, x86, debug, gpu> in /mnt/git/webkit-style-queue/LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:3924 Path does not exist. svg/filters/feImage-preserveAspectRatio.svg
LayoutTests/platform/chromium/test_expectations.txt:3924:  Path does not exist. svg/filters/feImage-preserveAspectRatio.svg  [test/expectations] [2]
ERROR: FAILURES FOR <lucid, x86, release, cpu> in /mnt/git/webkit-style-queue/LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:3924 Path does not exist. svg/filters/feImage-preserveAspectRatio.svg
LayoutTests/platform/chromium/test_expectations.txt:3924:  Path does not exist. svg/filters/feImage-preserveAspectRatio.svg  [test/expectations] [2]
ERROR: FAILURES FOR <lucid, x86, release, gpu> in /mnt/git/webkit-style-queue/LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:3924 Path does not exist. svg/filters/feImage-preserveAspectRatio.svg
LayoutTests/platform/chromium/test_expectations.txt:3924:  Path does not exist. svg/filters/feImage-preserveAspectRatio.svg  [test/expectations] [2]
ERROR: FAILURES FOR <lucid, x86_64, debug, cpu> in /mnt/git/webkit-style-queue/LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:3924 Path does not exist. svg/filters/feImage-preserveAspectRatio.svg
LayoutTests/platform/chromium/test_expectations.txt:3924:  Path does not exist. svg/filters/feImage-preserveAspectRatio.svg  [test/expectations] [2]
ERROR: FAILURES FOR <lucid, x86_64, debug, gpu> in /mnt/git/webkit-style-queue/LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:3924 Path does not exist. svg/filters/feImage-preserveAspectRatio.svg
LayoutTests/platform/chromium/test_expectations.txt:3924:  Path does not exist. svg/filters/feImage-preserveAspectRatio.svg  [test/expectations] [2]
ERROR: FAILURES FOR <lucid, x86_64, release, cpu> in /mnt/git/webkit-style-queue/LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:3924 Path does not exist. svg/filters/feImage-preserveAspectRatio.svg
LayoutTests/platform/chromium/test_expectations.txt:3924:  Path does not exist. svg/filters/feImage-preserveAspectRatio.svg  [test/expectations] [2]
ERROR: FAILURES FOR <lucid, x86_64, release, gpu> in /mnt/git/webkit-style-queue/LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:3924 Path does not exist. svg/filters/feImage-preserveAspectRatio.svg
LayoutTests/platform/chromium/test_expectations.txt:3924:  Path does not exist. svg/filters/feImage-preserveAspectRatio.svg  [test/expectations] [2]
Total errors found: 32 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Nikolas Zimmermann 2012-01-22 11:58:54 PST
Created attachment 123500 [details]
Patch v2
Comment 4 Zoltan Herczeg 2012-01-23 02:21:23 PST
Comment on attachment 123500 [details]
Patch v2

This patch looks good.

View in context: https://bugs.webkit.org/attachment.cgi?id=123500&action=review

> Source/WebCore/svg/graphics/filters/SVGFilter.h:44
> -    FloatRect mapLocalRectToAbsoluteRect(const FloatRect& rect) const { return m_absoluteTransform.mapRect(rect); }
> +    const AffineTransform& absoluteTransform() const { return m_absoluteTransform; }

Is this a purely costemit change? Its call sites still just call mapRect...
Comment 5 Zoltan Herczeg 2012-01-23 02:24:35 PST
cosmetic. I am too tired today...
Comment 6 Nikolas Zimmermann 2012-01-23 02:30:39 PST
(In reply to comment #4)
> (From update of attachment 123500 [details])
> This patch looks good.
> 
> View in context: https://bugs.webkit.org/attachment.cgi?id=123500&action=review
> 
> > Source/WebCore/svg/graphics/filters/SVGFilter.h:44
> > -    FloatRect mapLocalRectToAbsoluteRect(const FloatRect& rect) const { return m_absoluteTransform.mapRect(rect); }
> > +    const AffineTransform& absoluteTransform() const { return m_absoluteTransform; }
> 
> Is this a purely costemit change? Its call sites still just call mapRect...
Exactly. I now need direct access to absoluteTransform in another place, so it makes no sense to keep around the mapLocalRectToAbsoluteRect method anymore.
Comment 7 Zoltan Herczeg 2012-01-23 03:58:12 PST
Comment on attachment 123500 [details]
Patch v2

r=me
Comment 8 Nikolas Zimmermann 2012-01-23 04:43:10 PST
Committed r105612: <http://trac.webkit.org/changeset/105612>