Bug 163157

Summary:

Reduce code duplication between CG, Cairo, and Direct2D

Product:

WebKit

Reporter:

Brent Fulgham <bfulgham>

Component:

Layout and Rendering

Assignee:

Brent Fulgham <bfulgham>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, dino, simon.fraser, thorton, zalan

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

See Also:

https://bugs.webkit.org/show_bug.cgi?id=161817

Attachments:

Description	Flags
Patch	none
Patch	darin: review+, simon.fraser: commit-queue-

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	Details Formatted Diff Diff
Patch (15.00 KB, patch) 2016-10-07 17:43 PDT, Brent Fulgham	darin: review+ simon.fraser: commit-queue-	Details Formatted Diff Diff
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

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

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.