Bug 11596 – KCanvasFilters rework

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Nikolas Zimmermann]

Similir to the work in patch 11468 (platform/graphics addition), KCanvasFilters has to be reworked. It contains dozens of classes, cluttered in one file - same for KCanvasFilterQuartz.h/mm.

My proposal is: (already discussed with Oliver)
Add new directory platform/graphics/svg/filters, and filters/cg.

Split up KCanvasFilters in quite some new classes, and move them to platform/graphics/svg/filters.
Move the SVGResourceFilter class to platform/graphics/svg, similar to masker/clipper resources.

Add the cg implementations of the filter effects in platform/graphics/svg/filters/cg, named
SVGFETileCg.mm, SVGFEBlendCg.mm etc.

Move KCanvasPoint3F from kcanvas/device/KCanvasFilters.cpp/h to platform/graphics/FloatPoint3D.

Move everything from kcanvas/device/quartz/filters/* to platform/graphics/svg/filters/cg/.
(These are the cikernel files, all named Wk*Filter)

All these togeter kill KCanvasFilters.cpp/h and KCanvasFiltersQuartz.cpp/h, this is the last
file in the kcanvas/ directory, it's empty now. kcanvas/device is the next victim...

Attaching patch soon

[Nikolas Zimmermann]

Created attachment 11520 [details]
Initial patch

Large patch including ChangeLogs.
The commiter must move the Wk* files from kcanvas/device/quartz/filters to platform/graphics/svg/filters/cg (svn move!) this can't be included in the patch, it would add/remove these files and loose history.

Finally the duplicated SVGFilter* enums are gone, and shared between platform/graphics/svg/filters
and ksvg2/svg. All build system changes for Qt & Mac are included.

Have fun reviewing! :-)

[Darin Adler]

Do these really belong in the platform directory? I'm concerned that more and more of the SVG implementation is being moved into the platform directory.

[Nikolas Zimmermann]

Hey Darin,

that totally depends on how the WebKit structure should be in future.
For me platform/ means, platform specific implementations go there,
and for instance svg filters have to be implemented for every platform.

We could also have ksvg2/filters of course, with ksvg2/filters/cg,
ksvg2/filters/qt subdirectories. My personal opinion would be to
go with platform/graphics/svg for platform specific svg code.
If you have any worries with that, please elaborate :-)

Niko

[Nikolas Zimmermann]

Holding back commit for now, to give Darin a chance to comment.
Would be nice if you could manage to reply today (Friday), as I only
got hacking time today/tonite :-)

Thanks in advance,
Niko

[Darin Adler]

(In reply to comment #3)
> that totally depends on how the WebKit structure should be in future.
> For me platform/ means, platform specific implementations go there,
> and for instance svg filters have to be implemented for every platform.

That's not quite right.

We expect to have platform-specific implementations in some other places in the source tree, here and there. What goes into the platform directory is a platform abstraction that the rest of the code is built on. But in some cases, you can't build the platform specifics into a platform abstraction -- you need platform specifics in the actual high level code.

At the moment, the directories that are doing this are page and loader. (Much of the code currently in bridge/mac belongs in page/mac.) While we'd like to keep from doing this all over the code, we will need platform specifics outside the platform directory when they can't easily be abstracted into a generic cross-platform concept.

If we came up with a way to make filters a concept that was independent of SVG, then I think platform/graphics/filters would be a fine location.

But as long as the filter implementation is closely tied to the SVG implementation, even if it has platform-specific pieces, it should be in svg/filters, with svg/filters/ci or cg and other such subdirectories as needed to keep the platform-specific parts in their own subdirectories.

> We could also have ksvg2/filters of course, with ksvg2/filters/cg,
> ksvg2/filters/qt subdirectories. My personal opinion would be to
> go with platform/graphics/svg for platform specific svg code.
> If you have any worries with that, please elaborate :-)

I do have worries about that. This is not the original concept that Hyatt created about what the platform directory is.

I see signs that people are misunderstanding this. For example, FrameQt and PageQt absolutely do not belong in platform/qt. They should be in page/qt instead.

I'm sorry that this wasn't explained clearly enough in the past; we probably need to write more about it. I don't mean to make your work more difficult. Please talk to Hyatt and Maciej about this too as the three of us worked together on this design and strategy. I want others in the project to make comments about it too and help us get it just right.

[Nikolas Zimmermann]

Hi Darin,

fair enough, this is the first real explaination about platform/
that I've read - and I like it and agree with it.

Compromise: Can I commit this patch as-is, and move the _whole_
platform/graphics/svg directory into ksvg2 subdirectory?

With your explaination about platform/, the platform/graphics directory
makes a lot of sense, but platform/graphics/svg not.

You're okay with this?
Niko

[Darin Adler]

(In reply to comment #6)
> Compromise: Can I commit this patch as-is, and move the _whole_
> platform/graphics/svg directory into ksvg2 subdirectory?
> 
> With your explaination about platform/, the platform/graphics directory
> makes a lot of sense, but platform/graphics/svg not.
> 
> You're okay with this?

Sounds fine to me. What I care about most is the end point, not the path to get there.

[Nikolas Zimmermann]

Landed in r17850.