Summary: | Make FontCascade::CodePath an enum class | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rob Buis <rbuis> | ||||||||||||||
Component: | New Bugs | Assignee: | Rob Buis <rbuis> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | achristensen, changseok, darin, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, kondapallykalyan, mmaxfield, pdr, sabouhallawa, schenney, sergio, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Rob Buis
2020-12-16 06:46:51 PST
Created attachment 416336 [details]
Patch
Created attachment 416338 [details]
Patch
Comment on attachment 416338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416338&action=review > Source/WebKit/ChangeLog:11 > +2020-12-16 Rob Buis <rbuis@igalia.com> Double change log entry. > Source/WebCore/platform/graphics/FontCascade.h:-192 > - enum CodePath { Auto, Simple, Complex, SimpleWithGlyphOverflow }; This should probably remain a nested enum. FontCascade::CodePath::Auto Created attachment 416363 [details]
Patch
Comment on attachment 416338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416338&action=review >> Source/WebKit/ChangeLog:11 >> +2020-12-16 Rob Buis <rbuis@igalia.com> > > Double change log entry. Oops! Fixed. >> Source/WebCore/platform/graphics/FontCascade.h:-192 >> - enum CodePath { Auto, Simple, Complex, SimpleWithGlyphOverflow }; > > This should probably remain a nested enum. > FontCascade::CodePath::Auto A with verbose, but ok... Looks like you missed one in Source\WebCore\platform\win\WebCoreTextRenderer.cpp Created attachment 416398 [details]
Patch
Created attachment 416400 [details]
Patch
Committed r270932: <https://trac.webkit.org/changeset/270932> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416400 [details]. Comment on attachment 416400 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416400&action=review > Source/WebCore/platform/graphics/FontCascade.h:-41 > -// "X11/X.h" defines Complex to 0 and conflicts > -// with Complex value in CodePath enum. > -#ifdef Complex > -#undef Complex > -#endif This change seems unrelated to the rest. If there is a macro named Complex, then it’s just as much a problem with the enum class as it as with the enum. Reopening to attach new patch. Created attachment 416430 [details]
Patch
Comment on attachment 416400 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416400&action=review >> Source/WebCore/platform/graphics/FontCascade.h:-41 >> -#endif > > This change seems unrelated to the rest. If there is a macro named Complex, then it’s just as much a problem with the enum class as it as with the enum. The change was intentional on my part but wrong :( I prepared the partial revert patch, I am wondering if there is a rename possible for Complex, but I bet others have considered that in the past already, and I guess we need to keep it that way to match ComplexText in some method names. If GTK builds successfully without undefining Complex, let's just keep it removed. (In reply to Alex Christensen from comment #15) > If GTK builds successfully without undefining Complex, let's just keep it > removed. Exactly, but that can be done even if we don’t do anything else. FWIW CodePath::Complex prints as 2 on my Linux Ubuntu setup. I wonder if we include X11 headers in any configuration... Comment on attachment 416430 [details]
Patch
I guess we can do this, if we want to remove such code it should probably be a separate bug with a clear title anyway.
Committed r271784: <https://trac.webkit.org/changeset/271784> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416430 [details]. |