[webkit-reviews] review denied: [Bug 10696] RenderPathQuartz and RenderPathQt should not be needed : [Attachment 10371] first attempt

[bugzilla-request-daemon at opendarwin.org]

Eric Seidel <macdome at opendarwin.org> has denied Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 10696: RenderPathQuartz and RenderPathQt should not be needed
http://bugzilla.opendarwin.org/show_bug.cgi?id=10696

Attachment 10371: first attempt
http://bugzilla.opendarwin.org/attachment.cgi?id=10371&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
Unfortunately I lost all my good comments due to a Safari crash.  Here is an
abridged version:

1.  Lots of style nits.  Pointers in the "wrong" place, missing spaces between
ifs, {} misplaced.  While we're moving this code around we should fix the
style.

2.  if/elseif/else sets for enums are used where switches should be.  Also some
enum-based switches here use default: instead of mentioning each enum value. 
Using a switch statement for an enum and mentioning each enum value by name is
best as it allows for better compile-time error checking.

3. DrawMarkersData should use constructor-style initializers here:
+    elementIndex = 0;
+    midMarker = mid;
(yes, I know this is old code copied, but might as well fix it now.)

4.  createRoundedRectangle needs a bath:
+    double nrx = rx, nry = ry;
should be renamed (I have no clue what nrx is...)
0.552 should be turned into a nicely named constant (to make its use more
understandable.

Not needed:
+    // Ellipse creation - nice & clean agg2 code


+void CGPathTransformer(void *info, const CGPathElement *element)
 is completely unnecessary.  Use CGAddPath instead and pass a transform.

Lots of small stuff, but enough to add up to a r-.