Bug 21898

Summary: Remove unused TextDrawingMode flags from GraphicsContext
Product: WebKit Reporter: Brett Wilson (Google) <brettw>
Component: PlatformAssignee: Caio Marcelo de Oliveira Filho <cmarcelo>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cmarcelo, eric, senorblanco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch benjamin: review+

Brett Wilson (Google)
Reported 2008-10-27 09:07:28 PDT
I could not figure out how to handle this text drawing mode looking at the header. It appears that this value is never used, so it should be removed.
Attachments
Patch (4.95 KB, patch)
2012-08-21 06:04 PDT, Caio Marcelo de Oliveira Filho 		no flags
DetailsFormatted DiffDiff
Patch (4.89 KB, patch)
2012-08-21 06:38 PDT, Caio Marcelo de Oliveira Filho 		benjamin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Caio Marcelo de Oliveira Filho
Comment 1 2012-08-21 05:56:55 PDT
This was also suggested in this other bug comment https://bugs.webkit.org/show_bug.cgi?id=42110#c2
Caio Marcelo de Oliveira Filho
Comment 2 2012-08-21 05:57:20 PDT
This was also suggested in comment https://bugs.webkit.org/show_bug.cgi?id=42110#c2
Caio Marcelo de Oliveira Filho
Comment 3 2012-08-21 06:04:54 PDT
Created attachment 159673 [details] Patch
Caio Marcelo de Oliveira Filho
Comment 4 2012-08-21 06:38:43 PDT
Created attachment 159678 [details] Patch
Benjamin Poulain
Comment 5 2012-08-21 17:15:30 PDT
Comment on attachment 159678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159678&action=review Looks good, grep does not find those values used anywhere. > Source/WebCore/platform/graphics/GraphicsContext.h:-134 > - TextModeInvisible = 0, > TextModeFill = 1 << 0, > TextModeStroke = 1 << 1, > - TextModeClip = 1 << 2 Why not shift the values? TextModeFill = 0, TextModeStroke = 1 << 0
Caio Marcelo de Oliveira Filho
Comment 6 2012-08-21 17:36:49 PDT
Comment on attachment 159678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159678&action=review Thanks for the review. >> Source/WebCore/platform/graphics/GraphicsContext.h:-134 >> - TextModeClip = 1 << 2 > > Why not shift the values? > TextModeFill = 0, > TextModeStroke = 1 << 0 These values are used as bitflags in existing code. TextModeFill can't be zero wouldn't because we wouldn't be able to identify its presence using & binary operator.
Caio Marcelo de Oliveira Filho
Comment 7 2012-08-21 17:38:04 PDT
Comment on attachment 159678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159678&action=review >>> Source/WebCore/platform/graphics/GraphicsContext.h:-134 >>> - TextModeClip = 1 << 2 >> >> Why not shift the values? >> TextModeFill = 0, >> TextModeStroke = 1 << 0 > > These values are used as bitflags in existing code. TextModeFill can't be zero wouldn't because we wouldn't be able to identify its presence using & binary operator. Oh my, s/wouldn't//. :-P
Benjamin Poulain
Comment 8 2012-08-21 17:46:41 PDT
Comment on attachment 159678 [details] Patch > > Why not shift the values? > > TextModeFill = 0, > > TextModeStroke = 1 << 0 > > These values are used as bitflags in existing code. TextModeFill can't be zero because we wouldn't be able to identify its presence using & binary operator. Yep, that sounds ok. Feel free to cq+ if you haven't committed already.
Caio Marcelo de Oliveira Filho
Comment 9 2012-08-22 07:54:23 PDT
Committed r126302: <http://trac.webkit.org/changeset/126302>
Note You need to log in before you can comment on or make changes to this bug.