Summary: | [BREWMP] Port graphics backend | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kwang Yul Seo <skyul> | ||||||||||||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||||||||
Severity: | Normal | CC: | beergun, commit-queue, jamesr, joybro201, xhiloh | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | Other | ||||||||||||||||||
OS: | Other | ||||||||||||||||||
Bug Depends on: | |||||||||||||||||||
Bug Blocks: | 33564 | ||||||||||||||||||
Attachments: |
|
Description
Kwang Yul Seo
2010-06-07 18:55:37 PDT
Created attachment 65777 [details]
FontCustomPlatformData
Reuse chromium's FontCustomPlatformData.
CC'ing David Levin as it touches chromium. Comment on attachment 65777 [details] FontCustomPlatformData I will submit patches for WebKit Brew MP after bug 39672 is resolved. Created attachment 67622 [details]
FontCustomPlatformData
Add PLATFORM(BREWMP) guard to reuse the OS(LINUX) code.
CC'ing James here. Since Brew MP has no builders on build.webkit.org, what's the plan if changes to this file break that port? Also, what OS setting does Brew MP set? I don't know anything about it as a platform. Maybe it's simpler for Brew to just set OS(LINUX)? It's kind of weird to mix up OS() and PLATFORM() #ifdefs in a single expression. (In reply to comment #6) > Since Brew MP has no builders on build.webkit.org, what's the plan if changes to this file break that port? Brew MP uses waf. I am upstreaming the build system in bug 44645. Until we have a buildbot, I will fix the build manually :(. > Also, what OS setting does Brew MP set? I don't know anything about it as a platform. Maybe it's simpler for Brew to just set OS(LINUX)? It's kind of weird to mix up OS() and PLATFORM() #ifdefs in a single expression. A Brew MP application is written only with Brew MP API and C standard functions. Underlying OS is not exposed, so we don't know the operating system where Brew MP runs. Sadly, there is no way but to add PLATFORM(BREWMP) guard. Created attachment 68144 [details]
Font
Port Font to Brew MP.
Created attachment 68149 [details]
Font
Fix COMPILE_ASSERT.
(In reply to comment #6) > Since Brew MP has no builders on build.webkit.org, what's the plan if changes to this file break that port? > > Also, what OS setting does Brew MP set? I don't know anything about it as a platform. Maybe it's simpler for Brew to just set OS(LINUX)? It's kind of weird to mix up OS() and PLATFORM() #ifdefs in a single expression. Another way to clean up the code is to split OS(WINDOWS) and OS(LINUX) into separate files and move OS(WINDOWS) part to platform/graphics/chromium and leave OS(LINUX) part in platform/graphics/skia without guards. I think OS(WINDOWS) part of FontCustomPlatformData.cpp/h is still Chromium-specific while OS(LINUX) part depends only on skia. After this cleanup, Brew MP can use FontCustomPlatformData in platform/graphics/skia without modification. Created attachment 71186 [details]
FontCustomPlatformData
Update the patch because OS(FREEBSD) guard was added.
Created attachment 71219 [details]
FontPlatformData
Created attachment 71232 [details]
FontCache
(In reply to comment #13) > Created an attachment (id=71232) [details] > FontCache Except for FontCache::getFontDataForCharacters, all other methods are still the same as Chromium Linux. If you think it is better to share the code with PLATFORM(CHROMIUM) guards in FontCache::getFontDataForCharacters, I will submit an alternative patch in bug 39672. Comment on attachment 71186 [details]
FontCustomPlatformData
It seems like these files only handle two cases - windows (in which case they use GDI) and everything else (originally just linux, but later freebsd and now brewmp). Would it be useful to define another macro for the using-Skia-but-not-GDI case so we can stick the OS(LINUX) || OS(FREEBSD) || PLATFORM(BREWMP) logic all in one place?
Kwang, do you think such a macro would be useful in other files in addition to this pair? If so we should consider adding it, but if it's just for these files then I think this way is fine.
R=me, request commit-queue again if you want to land with these macros.
(In reply to comment #15) > (From update of attachment 71186 [details]) > It seems like these files only handle two cases - windows (in which case they use GDI) and everything else (originally just linux, but later freebsd and now brewmp). Would it be useful to define another macro for the using-Skia-but-not-GDI case so we can stick the OS(LINUX) || OS(FREEBSD) || PLATFORM(BREWMP) logic all in one place? > > Kwang, do you think such a macro would be useful in other files in addition to this pair? If so we should consider adding it, but if it's just for these files then I think this way is fine. > > R=me, request commit-queue again if you want to land with these macros. Currently, only there files have OS(LINUX) || OS(FREEBSD) || PLATFORM(BREWMP) check. Comment on attachment 71186 [details]
FontCustomPlatformData
OK!
Comment on attachment 71186 [details] FontCustomPlatformData Clearing flags on attachment: 71186 Committed r70675: <http://trac.webkit.org/changeset/70675> Comment on attachment 68149 [details]
Font
Is there anything that is specific to BREW? If not you should really share the implementation.
Comment on attachment 68149 [details]
Font
Is there anything that is specific to BREW? If not you should really share the implementation.
Comment on attachment 68149 [details]
Font
Is there anything that is specific to BREW? If not you should really share the implementation.
Comment on attachment 68149 [details]
Font
I agree to Holger. Looks like you should share the code than copying it.
Comment on attachment 71219 [details]
FontPlatformData
ditto.
Comment on attachment 71232 [details]
FontCache
ditto.
Brew MP port is no longer maintained. |