RESOLVED FIXED 163157
Reduce code duplication between CG, Cairo, and Direct2D 
https://bugs.webkit.org/show_bug.cgi?id=163157
Summary Reduce code duplication between CG, Cairo, and Direct2D
Brent Fulgham
Reported 2016-10-07 16:23:09 PDT
The various GraphicsContext implementations duplicate utility functions, constants, and other code. We should reduce this duplication to help ensure consistency and reduce maintenance burden.
Attachments
Patch (14.92 KB, patch)
2016-10-07 17:33 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (15.00 KB, patch)
2016-10-07 17:43 PDT, Brent Fulgham 		darin: review+
simon.fraser: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2016-10-07 17:33:28 PDT
Created attachment 290988 [details] Patch
Simon Fraser (smfr)
Comment 2 2016-10-07 17:40:46 PDT
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?
Brent Fulgham
Comment 3 2016-10-07 17:43:55 PDT
Created attachment 290990 [details] Patch
Brent Fulgham
Comment 4 2016-10-07 17:46:04 PDT
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.
Simon Fraser (smfr)
Comment 5 2016-10-07 22:51:52 PDT
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
Brent Fulgham
Comment 6 2016-10-10 09:48:31 PDT
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!
Brent Fulgham
Comment 7 2016-10-10 09:48:52 PDT
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.
Brent Fulgham
Comment 8 2016-10-10 10:05:27 PDT
Committed r207002: <http://trac.webkit.org/changeset/207002>
Darin Adler
Comment 9 2016-10-10 10:07:33 PDT
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.
Brent Fulgham
Comment 10 2016-10-10 10:15:03 PDT
Fixed typo in Committed r207003: <http://trac.webkit.org/changeset/207003>
Note You need to log in before you can comment on or make changes to this bug.