Bug 219947 – Make FontCascade::CodePath an enum class

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Rob Buis]

Make FontCascade::CodePath an enum class.

[Rob Buis]

Created attachment 416336 [details]
Patch

[Rob Buis]

Created attachment 416338 [details]
Patch

[Alex Christensen]

Comment on attachment 416338 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416338&action=review

> Source/WebKit/ChangeLog:11
> [phone]  Rob Buis  <[e-mail]>

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

[Rob Buis]

Created attachment 416363 [details]
Patch

[Rob Buis]

Comment on attachment 416338 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416338&action=review

>> Source/WebKit/ChangeLog:11
>> [phone]  Rob Buis  <[e-mail]>
> 
> 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...

[Alex Christensen]

Looks like you missed one in Source\WebCore\platform\win\WebCoreTextRenderer.cpp

[Rob Buis]

Created attachment 416398 [details]
Patch

[Rob Buis]

Created attachment 416400 [details]
Patch

[EWS]

Committed r270932: <https://trac.webkit.org/changeset/270932>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416400 [details].

[Radar WebKit Bug Importer]

<rdar://problem/72420593>

[Darin Adler]

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.

[Rob Buis]

Reopening to attach new patch.

[Rob Buis]

Created attachment 416430 [details]
Patch

[Rob Buis]

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.

[Alex Christensen]

If GTK builds successfully without undefining Complex, let's just keep it removed.

[Darin Adler]

(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.

[Rob Buis]

FWIW CodePath::Complex prints as 2 on my Linux Ubuntu setup. I wonder if we include X11 headers in any configuration...

[Rob Buis]

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.

[EWS]

Committed r271784: <https://trac.webkit.org/changeset/271784>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416430 [details].