Bug 48116 – [GTK] Move font related files.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Ryuan Choi]

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

[Ryuan Choi]

Created attachment 71537 [details]
Patch

[Ryuan Choi]

Created attachment 71552 [details]
Patch

[Antonio Gomes]

Comment on attachment 71552 [details]
Patch

Nice! Lets wait Gtk's EWS to catch up on this.

[Martin Robinson]

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.

[Martin Robinson]

> 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. :)

[Martin Robinson]

Switching review flag for the reason stated above.  You might want to coordinate with Brent Fulgham on how best to position the WinCairo include.

[Ryuan Choi]

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

[Ryuan Choi]

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.

[Adam Barth]

Comment on attachment 71676 [details]
PatchConsideringWinCairo

Looks like the gtk-ews is happy with this change.  Shall we rock and roll?

[Martin Robinson]

Brent Fulgham said he would take a look at this change today (hopefully).  It pretty seriously affects the WinCairo port, which has no EWS.

[Martin Robinson]

Well, it's been quite some time and there has been no objections, so I think we should just commit this.

[Martin Robinson]

Ryuan, FontPlatformDataFreetype has changed quite a bit since you posted this patch -- do you mind posting an updated version?

[Ryuan Choi]

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

[Ryuan Choi]

Created attachment 73577 [details]
Patch

[Ryuan Choi]

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

[WebKit Commit Bot]

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 [e-mail] and [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

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 [e-mail] and [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 73577 [details]
Patch

Clearing flags on attachment: 73577

Committed r71816: <http://trac.webkit.org/changeset/71816>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Roben (:aroben)]

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.

[Adam Roben (:aroben)]

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

[Adam Roben (:aroben)]

I'm going to take care of fixing the build. I'm going to move win/FontPlatformData.h to win/cairo/FontPlatformData.h.

[Brent Fulgham]

That sounds good to me.