Bug 149036 – Split FontDescription into lower and higher level types

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Antti Koivisto]

Currently FontDescription is used through the text subsystem. However much of the data it carries is only needed by FontCascade and text layout but not by the lower Font/FontCache layer. This make code confusing. For example families specified in FontDescription are ignored at lower levels.

[Antti Koivisto]

Created attachment 260939 [details]
patch

[Myles C. Maxfield]

Too much red to review.

[Antti Koivisto]

Created attachment 260998 [details]
patch

[Antti Koivisto]

Created attachment 261002 [details]
patch

[Darin Adler]

Comment on attachment 261002 [details]
patch

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

A bit of a shame that the more commonly used class is the one that has a longer name now.

> Source/WebCore/platform/graphics/FontDescription.h:41
>  class FontDescription {

Seems like this class is almost a struct.

> Source/WebCore/platform/graphics/FontDescription.h:50
> +    int computedPixelSize() const { return int(m_computedSize + 0.5f); }

Not new: This is a strange way to write rounding code.

> Source/WebCore/platform/graphics/FontDescription.h:66
> +    void setComputedSize(float s) { m_computedSize = clampToFloat(s); }

Maybe not new: I don’t understand why it’s valuable to call clampToFloat on a float.

> Source/WebCore/platform/graphics/FontDescription.h:82
> +    FontTraitsMask traitsMask() const;
> +private:

Normally we leave a blank line before "private".

> Source/WebCore/rendering/RenderFullScreen.cpp:88
> +    fullscreenStyle.get().setFontDescription(FontCascadeDescription());

Might want to use { } here so we don’t have to state the type explicitly.

[Antti Koivisto]

Created attachment 261239 [details]
patch

[Antti Koivisto]

Created attachment 261249 [details]
patch

[WebKit Commit Bot]

Comment on attachment 261249 [details]
patch

Clearing flags on attachment: 261249

Committed r189830: <http://trac.webkit.org/changeset/189830>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.