[Webkit-unassigned] [Bug 22902] RenderPath clean-up for strokeBoundingBox
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Dec 21 12:40:56 PST 2008
https://bugs.webkit.org/show_bug.cgi?id=22902
zimmermann at kde.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #26187| |review-
Flag| |
------- Comment #7 from zimmermann at kde.org 2008-12-21 12:40 PDT -------
(From update of attachment 26187)
Nice patch, overall, r- due some subtle issues:
> Index: WebCore/platform/graphics/StrokeStyleApplier.h
> + class StrokeStyleApplier {
> + public:
> + virtual ~StrokeStyleApplier() {}
> + virtual void strokeStyle(GraphicsContext*) = 0;
> + };
> +}
I'd rather move the destructor in a protected section, and also add a
constructor - protected as well.
So only the classes deriving from StrokeStyleApplier can create/destruct it.
> Index: WebCore/platform/graphics/cg/PathCG.cpp
> +static CGContextRef createScratchContext()
> +{
> + static CGContextRef s_contextRef = 0;
> + if (!s_contextRef) {
> + CFMutableDataRef empty = CFDataCreateMutable(NULL, 0);
> + CGDataConsumerRef consumer = CGDataConsumerCreateWithCFData(empty);
> + s_contextRef = CGPDFContextCreate(consumer, NULL, NULL);
> + CGDataConsumerRelease(consumer);
> + CFRelease(empty);
> +
> + CGFloat black[4] = {0, 0, 0, 1};
> + CGContextSetFillColor(s_contextRef, black);
> + CGContextSetStrokeColor(s_contextRef, black);
> + }
> + return s_contextRef;
> +}
I think, what Darin intented is to have a createScratchContext() class that
only creates the CGContextRef, it is not supposed to store it.
So just rename "static CGContextRef s_contextRef" -> "CGContextRef contextRef",
and return that.
> Index: WebCore/rendering/RenderPath.cpp
> +class StrokeBoundingRectStyleApplier : public StrokeStyleApplier {
Sorry that I suggested this bad name :-) I think "StrokeStyle" should remain as
one word, so "BoundingRectStrokeStyleApplier" may sounds a bit more approriate.
Can you clean the review flag of the patch Darin already r+ed?
Keen to review the final version :-)
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
More information about the webkit-unassigned
mailing list