Bug 4459 - More kcanvas improvements
Summary: More kcanvas improvements
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Tobias Lidskog
Depends on:
Reported: 2005-08-16 15:28 PDT by Tobias Lidskog
Modified: 2005-08-19 15:03 PDT (History)
0 users

See Also:

kcanvas patch (6.05 KB, patch)
2005-08-16 15:29 PDT, Tobias Lidskog
no flags Details | Formatted Diff | Diff
fixed bug (5.72 KB, patch)
2005-08-16 17:41 PDT, Tobias Lidskog
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Lidskog 2005-08-16 15:28:28 PDT
A few minor improvements to kcanvas that can improve performance quite a bit on complex svg and also 
fixes a bug or two in the hit testing code.
Comment 1 Tobias Lidskog 2005-08-16 15:29:11 PDT
Created attachment 3429 [details]
kcanvas patch

Some comments about the changes (performance improvements measured with Shark
and window resize tests with complex svg files. All render tree test pass, with
the exception of metadata-example-01-b that has to be patched):

KCanvasContainer::bbox(bool includeStroke)
The if inside the loop is only used for the first item, no need to check the if
during each iteration. QRect::unite already disregards empty rects, so there
shouldn't be any functional change. 

KCanvasContainer::collisions(const QPoint &p, KCanvasItemList &hits)
Don't return after checking last container (fix from kde TOT). Also check
against the correct bounding boxes for fill and stroke, and make the cheaper
rect compares before the fillContains and strokeContains.

KCanvasItemQuartz::draw(const QRect &rect)
Use bbox(true) instead of bboxPath(true, false) since bbox(true) calls
bboxPath(true, false), but also uses caching.
Check if the are any clip paths before calling applyClipPathsForStyle to avoid
calculating bboxPath(true) as part of the arguments. Most of the time there is
no clipping, and bboxPath(true) is an expensive call.

The changes in KCanvasContainer::bbox result in a container with nothing but
empty children will be positioned at the first child, not the last. Note that
these polygons should probably not be size 0x0, but that's another bug.
Comment 2 Tobias Lidskog 2005-08-16 15:32:28 PDT
forgot to mention one change in KCanvasItemQuartz::hitsPath - fill hit testing takes transformation into 
account, stroke hit testing should too
Comment 3 Tobias Lidskog 2005-08-16 17:41:32 PDT
Created attachment 3430 [details]
fixed bug

remove one bad change (in KCanvasItemQuartz::draw) mentioned by Eric on irc
Comment 4 Eric Seidel (no email) 2005-08-17 01:05:35 PDT
Comment on attachment 3430 [details]
fixed bug

Looks great.