Bug 12053 - SVGPathSeg*(Abs|Rel) classses should be combined to reduce code size
Summary: SVGPathSeg*(Abs|Rel) classses should be combined to reduce code size
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P4 Trivial
Assignee: Nobody
Keywords: EasyFix
Depends on:
Reported: 2006-12-31 19:47 PST by Eric Seidel (no email)
Modified: 2008-05-25 10:36 PDT (History)
0 users

See Also:

First attempt (48.15 KB, patch)
2008-05-18 09:50 PDT, Rob Buis
rwlbuis: review-
Details | Formatted Diff | Diff
Improved patch (57.43 KB, patch)
2008-05-22 14:00 PDT, Rob Buis
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2006-12-31 19:47:21 PST
SVGPathSeg*(Abs|Rel) classses should be combined to reduce code size

It's silly to have copy/paste copies of each Rel/Abs SVGPathSeg subclass.  These could be cleaned up to use templates, or even just a simple shared base class to reduce the amount of copy/paste code (and thus help prevent errors when changing these files).
Comment 1 Rob Buis 2008-05-18 09:50:39 PDT
Created attachment 21221 [details]
First attempt

Simple cleanup patch. 

Comment 2 Eric Seidel (no email) 2008-05-20 14:03:17 PDT
Comment on attachment 21221 [details]
First attempt

+        virtual String toString() const { return pathSegTypeAsLetter() + String::format(" %.6lg %.6lg", m_x, m_y); }

Might be more efficient if you passed the char as part of the format.  Not sure it matters though.

Generally we encourage each of these to get their own line:
 36         : m_x(x), m_y(y), m_r1(r1), m_r2(r2), m_angle(angle), m_largeArcFlag(largeArcFlag), m_sweepFlag(sweepFlag) {}

In general looks great!  I love all the minus lines.
Comment 3 Rob Buis 2008-05-22 14:00:14 PDT
Created attachment 21303 [details]
Improved patch

So I noticed that the template approach added some bloat, I think we are better
without it. Also I missed two files, now they are part of the patch.

Comment 4 Eric Seidel (no email) 2008-05-22 14:11:39 PDT
Comment on attachment 21303 [details]
Improved patch

Looks fine.
Comment 5 Rob Buis 2008-05-25 10:36:59 PDT
Landed in r34117.