Bug 15177

Summary:

[gtk] FontPlatformData contains more fields than required

Product:

WebKit

Reporter:

Sven Herzberg <sven>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

richard

Priority:

Version:

523.x (Safari 3)

Hardware:

OS:

Linux

Attachments:

Description	Flags
0001 - Made setFont() easier	none
0002 - Removed m_fontFace	none
0003 - Removed m_fontMatrix	none
0004 - Removed m_options	none
Final Patch (all in one)	mrowe: review+

Sven Herzberg

Reported 2007-09-11 07:32:38 PDT

FontPlatformData::setFont(cairo_t*) can be changed to use m_scaledFont directly. This makes the function easier readable and still works exactly as expected. m_fontFace, m_fontMatrix, and m_options are used in the main constructor in FontPlatformDataGdk.cpp only then. So they should be turned into local variables instead of extra fields. The attached patch-set reduces the memory footprint of each FontPlatformData by 3*sizeof(void*). The only issue that's left is that the former fields are not renamed according the the code conventions in my patches, do you want another patch for that?

Attachments
0001 - Made setFont() easier (1.19 KB, patch) 2007-09-11 07:34 PDT, Sven Herzberg	no flags	Details Formatted Diff Diff
0002 - Removed m_fontFace (3.31 KB, patch) 2007-09-11 07:35 PDT, Sven Herzberg	no flags	Details Formatted Diff Diff
0003 - Removed m_fontMatrix (3.67 KB, patch) 2007-09-11 07:36 PDT, Sven Herzberg	no flags	Details Formatted Diff Diff
0004 - Removed m_options (3.14 KB, patch) 2007-09-11 07:36 PDT, Sven Herzberg	no flags	Details Formatted Diff Diff
Final Patch (all in one) (4.52 KB, patch) 2007-09-11 07:58 PDT, Sven Herzberg	mrowe: review+	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Sven Herzberg

Comment 1 2007-09-11 07:34:55 PDT

Created attachment 16250 [details] 0001 - Made setFont() easier

Sven Herzberg

Comment 2 2007-09-11 07:35:28 PDT

Created attachment 16251 [details] 0002 - Removed m_fontFace

Sven Herzberg

Comment 3 2007-09-11 07:36:09 PDT

Created attachment 16252 [details] 0003 - Removed m_fontMatrix

Sven Herzberg

Comment 4 2007-09-11 07:36:49 PDT

Created attachment 16253 [details] 0004 - Removed m_options This is the last patch for now…

Sven Herzberg

Comment 5 2007-09-11 07:37:57 PDT

This is the diffstat output: FontDataGdk.cpp | 6 ------ FontPlatformData.h | 11 +---------- FontPlatformDataGdk.cpp | 17 ++++++++--------- 3 files changed, 9 insertions(+), 25 deletions(-)

Sven Herzberg

Comment 6 2007-09-11 07:58:50 PDT

Created attachment 16254 [details] Final Patch (all in one) Final patch (included the 4 previous ones plus a changelog entry)

Mark Rowe (bdash)

Comment 7 2007-09-11 08:03:10 PDT

Comment on attachment 16254 [details] Final Patch (all in one) Some minor comments: The local variables inside FontPlatformData::FontPlatformData should lose the m_ prefix, which is used only for members. I think the declarations can be moved to their point of first use too. Our coding style also requires no space between the function name and it's arguments. I'll make those minor tweaks when I land this.

Mark Rowe (bdash)

Comment 8 2007-09-11 08:11:06 PDT

Landed in r25489.

Note You need to log in before you can comment on or make changes to this bug.