Bug 48116 - [GTK] Move font related files.
Summary: [GTK] Move font related files.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 46029
  Show dependency treegraph
 
Reported: 2010-10-22 00:20 PDT by Ryuan Choi
Modified: 2010-11-11 11:50 PST (History)
5 users (show)

See Also:


Attachments
Patch (129.82 KB, patch)
2010-10-22 02:02 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (128.27 KB, patch)
2010-10-22 05:23 PDT, Ryuan Choi
mrobinson: review-
Details | Formatted Diff | Diff
PatchConsideringWinCairo (136.19 KB, patch)
2010-10-24 02:06 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (138.85 KB, patch)
2010-11-10 21:13 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2010-10-22 00:20:05 PDT
I'll moved font related files like below,
as following a guide of mrobinson at https://bugs.webkit.org/show_bug.cgi?id=46029

freetype related file -> platform/graphics/freetype
pango related file -> platform/graphics/pango
remove FontPlatformData.h
Comment 1 Ryuan Choi 2010-10-22 02:02:28 PDT
Created attachment 71537 [details]
Patch
Comment 2 Ryuan Choi 2010-10-22 05:23:29 PDT
Created attachment 71552 [details]
Patch
Comment 3 Antonio Gomes 2010-10-22 07:02:14 PDT
Comment on attachment 71552 [details]
Patch

Nice! Lets wait Gtk's EWS to catch up on this.
Comment 4 Martin Robinson 2010-10-22 08:32:20 PDT
Comment on attachment 71552 [details]
Patch

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

> WebCore/GNUmakefile.am:3690
> +webcoregtk_cppflags += -DUSE_FREETYPE=1 \
> +                       -I$(srcdir)/WebCore/platform/graphics/freetype

Tiny nit, but to maintain consistency with the rest of the file, this should be:

webcoregt_cppflags += \
<hardtab>-DUSE_FREETYPE=1 \
<hardtab>-I$(srcdir)/WebCore/platform/graphics/freetype

I think this is going to break the WinCairo build. You have moved FontPlatformDataWinCairo.h to FontPlatformData.h somewhere in the include path for WinCairo.
Comment 5 Martin Robinson 2010-10-22 08:32:51 PDT
> I think this is going to break the WinCairo build. You have moved FontPlatformDataWinCairo.h to FontPlatformData.h somewhere in the include path for WinCairo.

Have _not_ moved. :)
Comment 6 Martin Robinson 2010-10-23 00:58:10 PDT
Switching review flag for the reason stated above.  You might want to coordinate with Brent Fulgham on how best to position the WinCairo include.
Comment 7 Ryuan Choi 2010-10-23 03:24:13 PDT
(In reply to comment #6)
> Switching review flag for the reason stated above.  You might want to coordinate with Brent Fulgham on how best to position the WinCairo include.

NP, Now I'm preparing windows environment to check WinCairo.
But It takes some times.
If possible, please let me know what should I do.
Comment 8 Ryuan Choi 2010-10-24 02:06:29 PDT
Created attachment 71676 [details]
PatchConsideringWinCairo

I moved win/FontPlatformDataCairoWin.h to win/FontPlatformData.h and change WebCore.vcproj
but It was predicted because I am still struggling with windows environment.

please let me know whether it was right.

ps> I changed GNUMakefile.am also like mentioned.
Comment 9 Adam Barth 2010-10-27 13:11:27 PDT
Comment on attachment 71676 [details]
PatchConsideringWinCairo

Looks like the gtk-ews is happy with this change.  Shall we rock and roll?
Comment 10 Martin Robinson 2010-10-27 13:16:37 PDT
Brent Fulgham said he would take a look at this change today (hopefully).  It pretty seriously affects the WinCairo port, which has no EWS.
Comment 11 Martin Robinson 2010-11-10 18:13:27 PST
Well, it's been quite some time and there has been no objections, so I think we should just commit this.
Comment 12 Martin Robinson 2010-11-10 18:14:37 PST
Ryuan, FontPlatformDataFreetype has changed quite a bit since you posted this patch -- do you mind posting an updated version?
Comment 13 Ryuan Choi 2010-11-10 18:33:05 PST
(In reply to comment #12)
> Ryuan, FontPlatformDataFreetype has changed quite a bit since you posted this patch -- do you mind posting an updated version?

sure, I'll do that now.
Comment 14 Ryuan Choi 2010-11-10 21:13:43 PST
Created attachment 73577 [details]
Patch
Comment 15 Ryuan Choi 2010-11-10 21:15:40 PST
(In reply to comment #14)
> Created an attachment (id=73577) [details]
> Patch

I updated patch after checking GTK+ port(both freetype and pango) and EFL port.
Comment 16 WebKit Commit Bot 2010-11-10 23:22:09 PST
The commit-queue encountered the following flaky tests while processing attachment 73577 [details]:

fast/workers/storage/use-same-database-in-page-and-workers.html
fast/css/font-face-download-error.html

Please file bugs against the tests.  These tests were authored by dumi@chromium.org and yuzo@google.com.  The commit-queue is continuing to process your patch.
Comment 17 WebKit Commit Bot 2010-11-11 06:31:50 PST
The commit-queue encountered the following flaky tests while processing attachment 73577 [details]:

http/tests/appcache/top-frame-3.html

Please file bugs against the tests.  These tests were authored by ap@webkit.org and dimich@chromium.org.  The commit-queue is continuing to process your patch.
Comment 18 WebKit Commit Bot 2010-11-11 06:33:40 PST
Comment on attachment 73577 [details]
Patch

Clearing flags on attachment: 73577

Committed r71816: <http://trac.webkit.org/changeset/71816>
Comment 19 WebKit Commit Bot 2010-11-11 06:33:46 PST
All reviewed patches have been landed.  Closing bug.
Comment 20 Adam Roben (:aroben) 2010-11-11 06:52:47 PST
This broke the Windows build (see <http://build.webkit.org/builders/Windows%20Debug%20%28Build%29/builds/27325/steps/compile-webkit/logs/stdio>). It looks like the Cairo-specific FontPlatformData.h is overwriting the CG-specific one that we use on Windows.
Comment 21 Adam Roben (:aroben) 2010-11-11 07:02:17 PST
(In reply to comment #20)
> It looks like the Cairo-specific FontPlatformData.h is overwriting the CG-specific one that we use on Windows.

Here's a little more context:

The WebCoreGenerated project copies WebCore's headers into $WebKitOutputDir, where they are picked up by other projects (specifically, WebKit). It copies headers from a bunch of different directories. Here are the lines that are relevant to the build failure:

xcopy /y /d "%ProjectDir%..\platform\graphics\*.h" "%WebKitOutputDir%\include\WebCore"
xcopy /y /d "%ProjectDir%..\platform\graphics\%1\*.h" "%WebKitOutputDir%\include\WebCore"
xcopy /y /d "%ProjectDir%..\platform\graphics\transforms\*.h" "%WebKitOutputDir%\include\WebCore"
xcopy /y /d "%ProjectDir%..\platform\graphics\win\*.h" "%WebKitOutputDir%\include\WebCore"

%1 is either "cairo" or "cg", depending on which configuration is being built.

So you can see that cairo/cg headers are copied first, then win headers are copied later. If the header names conflict, whichever header is newer will win (thanks to the /d flag). This patch made the Cairo-specific win/FontPlatformData.h header newer than the CG-specific one.

It seems wrong for a Cairo-specific file to be sitting in win/ without any mention of "cairo" in its name. Maybe it should be in a new win/cairo directory, or cairo/win?
Comment 22 Adam Roben (:aroben) 2010-11-11 07:36:31 PST
I'm going to take care of fixing the build. I'm going to move win/FontPlatformData.h to win/cairo/FontPlatformData.h.
Comment 23 Brent Fulgham 2010-11-11 11:50:23 PST
That sounds good to me.