| Summary: | <feImage> has problems referencing local elements | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikolas Zimmermann <zimmermann> | ||||||
| Component: | SVG | Assignee: | Nikolas Zimmermann <zimmermann> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | kling, krit, rwlbuis, webkit.review.bot, zherczeg, zimmermann | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 10403 | ||||||||
| Attachments: |
|
||||||||
Created attachment 123498 [details]
Patch
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.
Created attachment 123500 [details]
Patch v2
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... cosmetic. I am too tired today... (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 on attachment 123500 [details]
Patch v2
r=me
Committed r105612: <http://trac.webkit.org/changeset/105612> |
<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.