The various GraphicsContext implementations duplicate utility functions, constants, and other code. We should reduce this duplication to help ensure consistency and reduce maintenance burden.
Created attachment 290988 [details] Patch
Comment on attachment 290988 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290988&action=review > Source/WebCore/platform/graphics/GraphicsContext.cpp:1132 > + return strokeStyle() == DottedStroke ? thickness : std::min(2.0f * thickness, std::max(thickness, strokeWidth / 3.0f)); We should make a constrainedBetween<> template. > Source/WebCore/platform/graphics/GraphicsContext.cpp:1139 > + Blank line. > Source/WebCore/platform/graphics/GraphicsContext.h:624 > + float dashedLineCornerWidthForStrokeWidth(float); > + float dashedLinePatternWidthForStrokeWidth(float); > + float dashedLinePatternOffsetForPatternAndStrokeWidth(float patternWidth, float strokeWidth); > + Vector<FloatPoint> centerLineAndCutOffCorners(bool isVerticalLine, FloatPoint point1, FloatPoint point2); Can these all be static?
Created attachment 290990 [details] Patch
Comment on attachment 290988 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290988&action=review >> Source/WebCore/platform/graphics/GraphicsContext.cpp:1132 >> + return strokeStyle() == DottedStroke ? thickness : std::min(2.0f * thickness, std::max(thickness, strokeWidth / 3.0f)); > > We should make a constrainedBetween<> template. I wonder if we have one? I'll go look. >> Source/WebCore/platform/graphics/GraphicsContext.cpp:1139 >> + > > Blank line. Whoops! >> Source/WebCore/platform/graphics/GraphicsContext.h:624 >> + Vector<FloatPoint> centerLineAndCutOffCorners(bool isVerticalLine, FloatPoint point1, FloatPoint point2); > > Can these all be static? They could; I'd just need to pass the stroke style in, and maybe the stroke thickness.
Comment on attachment 290990 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290990&action=review > Source/WebCore/platform/graphics/GraphicsContext.cpp:1139 > + Blank line > Source/WebCore/platform/graphics/GraphicsContext.h:624 > + float dashedLineCornerWidthForStrokeWidth(float); > + float dashedLinePatternWidthForStrokeWidth(float); > + float dashedLinePatternOffsetForPatternAndStrokeWidth(float patternWidth, float strokeWidth); > + Vector<FloatPoint> centerLineAndCutOffCorners(bool isVerticalLine, float cornerWidth, FloatPoint point1, FloatPoint point2); These can all be const
Comment on attachment 290990 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290990&action=review >> Source/WebCore/platform/graphics/GraphicsContext.cpp:1139 >> + > > Blank line OK!
Comment on attachment 290990 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290990&action=review >> Source/WebCore/platform/graphics/GraphicsContext.h:624 >> + Vector<FloatPoint> centerLineAndCutOffCorners(bool isVerticalLine, float cornerWidth, FloatPoint point1, FloatPoint point2); > > These can all be const Done.
Committed r207002: <http://trac.webkit.org/changeset/207002>
Comment on attachment 290990 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290990&action=review > Source/WebCore/platform/graphics/GraphicsContext.cpp:1167 > + // Center line and cut off corners for pattern patining. Noticed a typo here: patining.
Fixed typo in Committed r207003: <http://trac.webkit.org/changeset/207003>