Bug 5862 - feDisplacementMap filter is not implemented
Summary: feDisplacementMap filter is not implemented
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks: 5857
  Show dependency treegraph
 
Reported: 2005-11-28 17:21 PST by Oliver Hunt
Modified: 2006-02-25 14:23 PST (History)
0 users

See Also:


Attachments
First patch for displacement mapping (26.75 KB, patch)
2006-02-08 18:10 PST, Oliver Hunt
no flags Details | Formatted Diff | Diff
First patch for displacement mapping (26.75 KB, patch)
2006-02-08 18:10 PST, Oliver Hunt
no flags Details | Formatted Diff | Diff
Formatting changes (26.76 KB, patch)
2006-02-08 18:29 PST, Oliver Hunt
eric: review-
Details | Formatted Diff | Diff
A few more formatting changes (27.86 KB, patch)
2006-02-09 18:40 PST, Oliver Hunt
oliver: review-
Details | Formatted Diff | Diff
Fixed filter kernel (27.87 KB, patch)
2006-02-11 17:46 PST, Oliver Hunt
no flags Details | Formatted Diff | Diff
Updated to patch against ToT (28.18 KB, patch)
2006-02-11 19:00 PST, Oliver Hunt
eric: review+
Details | Formatted Diff | Diff
Testcase (1.34 KB, image/svg+xml)
2006-02-11 19:01 PST, Oliver Hunt
no flags Details
displacement map for testcase (260 bytes, image/png)
2006-02-11 19:02 PST, Oliver Hunt
no flags Details
Source image for testcase (260 bytes, image/png)
2006-02-11 19:03 PST, Oliver Hunt
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2005-11-28 17:21:33 PST
 
Comment 1 Oliver Hunt 2006-02-08 18:10:45 PST
Created attachment 6352 [details]
First patch for displacement mapping

Okay this is the first version of the feDisplacementMap implementation.  Filter should be correct (per logic + quartz composer) however it's difficult to validate given test case (http://www.w3.org/Graphics/SVG/Test/20030813/htmlframe/full-filters-displace-01-f.html) fails to render the inputs correctly, let alone the output.  That said sphere map appears to be doing correct thing on page.
Comment 2 Oliver Hunt 2006-02-08 18:10:46 PST
Created attachment 6353 [details]
First patch for displacement mapping

Okay this is the first version of the feDisplacementMap implementation.  Filter should be correct (per logic + quartz composer) however it's difficult to validate given test case (http://www.w3.org/Graphics/SVG/Test/20030813/htmlframe/full-filters-displace-01-f.html) fails to render the inputs correctly, let alone the output.  That said sphere map appears to be doing correct thing on page.
Comment 3 Oliver Hunt 2006-02-08 18:29:22 PST
Created attachment 6354 [details]
Formatting changes

Damn formatting changes
Comment 4 Eric Seidel (no email) 2006-02-08 18:49:49 PST
Comment on attachment 6354 [details]
Formatting changes

This patch looks fine.

Ideally, we would want to remove the data duplication in the kcanvas objects, by making the KCanvasFEDisplacementMap object just fetch its needed data off of it's element() instead of storing a second copy locally.

Also, there are a couple tabs in the cikernel file which need to be fixed before we land.

also XChannelSelector() and m_XChannelSelector should be xChannelSelector() and m_xChannelSelector per our style guidelines.  (Or just completely removed per my above comment)

It also doesn't look like KCanvasFEDisplacementMap has a default constructor which sets things to sane default values (also unecessary if its data members are removed)

getVectorForChannel could be done using an array of floats of 0's, followed by setting one at the correct offset to 1 and returning.  Your method is also totally fine.

Since you don't have commit bit, I'm going to mark this as r-.  If you did, it would be find to land, making those tweaks as you landed.  This does not need another review, but a final patch should be posted.
Comment 5 Oliver Hunt 2006-02-09 18:40:43 PST
Created attachment 6375 [details]
A few more formatting changes
Comment 6 Eric Seidel (no email) 2006-02-09 21:07:47 PST
Comment on attachment 6375 [details]
A few more formatting changes

Fabulous, as always. r=me.
Comment 7 Oliver Hunt 2006-02-11 17:46:47 PST
Created attachment 6417 [details]
Fixed filter kernel

oops... correct the kernel so it works when the dispalcement map isn't completely symmetrical :)
Comment 8 Eric Seidel (no email) 2006-02-11 17:55:52 PST
Comment on attachment 6417 [details]
Fixed filter kernel

great!
Comment 9 Eric Seidel (no email) 2006-02-11 18:00:02 PST
Comment on attachment 6417 [details]
Fixed filter kernel

Hum... doens't apply cleanly though.  Let's get one more patch.  And I'll land your test case at that time too.
Comment 10 Oliver Hunt 2006-02-11 19:00:47 PST
Created attachment 6418 [details]
Updated to patch against ToT
Comment 11 Oliver Hunt 2006-02-11 19:01:43 PST
Created attachment 6419 [details]
Testcase
Comment 12 Oliver Hunt 2006-02-11 19:02:27 PST
Created attachment 6420 [details]
displacement map for testcase
Comment 13 Oliver Hunt 2006-02-11 19:03:24 PST
Created attachment 6421 [details]
Source image for testcase
Comment 14 Eric Seidel (no email) 2006-02-12 04:57:17 PST
Comment on attachment 6418 [details]
Updated to patch against ToT

Looks good.  Lets land this.