Bug 86233 - Remove bbox caching from SVGPathElement
Summary: Remove bbox caching from SVGPathElement
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
Depends on:
Reported: 2012-05-11 10:56 PDT by Rob Buis
Modified: 2012-05-19 13:55 PDT (History)
1 user (show)

See Also:

Patch (3.33 KB, patch)
2012-05-11 10:58 PDT, Rob Buis
zimmermann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2012-05-11 10:56:02 PDT
I dont think this code adds much value, as SVGLocatable::getBBox is not used much out there.
Comment 1 Rob Buis 2012-05-11 10:58:04 PDT
Created attachment 141450 [details]
Comment 2 Rob Buis 2012-05-11 18:29:51 PDT
I ran run-perf-tests on the svg PageLoad performance tests and there were no speed regressions there (makes sense since the changed code path is not called by these heavy svg tests). From our layout tests only svg/custom/getBBox-path.svg uses it, and there it is only asked once, so caching is not useful for this test. So overall I think the benefits outweigh the drawbacks of removing this caching, I just don't see this as a hot code path.
Comment 3 Nikolas Zimmermann 2012-05-19 08:23:53 PDT
Comment on attachment 141450 [details]

Good idea, this was more beneficial in the past where renderers grabbed bounds using getBBox().
Comment 4 Rob Buis 2012-05-19 13:55:24 PDT
Committed r117696: <http://trac.webkit.org/changeset/117696>