Bug 40275 - [BREWMP] Port graphics backend
: [BREWMP] Port graphics backend
Status: RESOLVED WONTFIX
Product: WebKit
Classification: Unclassified
Component: Platform
: 528+ (Nightly build)
: Other Other
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks: 33564
  Show dependency treegraph
 
Reported: 2010-06-07 18:55 PDT by Kwang Yul Seo
Modified: 2012-07-26 05:20 PDT (History)
5 users (show)

See Also:


Attachments
FontCustomPlatformData (23.99 KB, patch)
2010-08-27 15:47 PDT, Kwang Yul Seo
no flags Details | Formatted Diff | Diff
FontCustomPlatformData (4.61 KB, patch)
2010-09-14 17:34 PDT, Kwang Yul Seo
no flags Details | Formatted Diff | Diff
Font (8.43 KB, patch)
2010-09-20 15:20 PDT, Kwang Yul Seo
no flags Details | Formatted Diff | Diff
Font (8.45 KB, patch)
2010-09-20 15:40 PDT, Kwang Yul Seo
krit: review-
Details | Formatted Diff | Diff
FontCustomPlatformData (4.94 KB, patch)
2010-10-19 11:02 PDT, Kwang Yul Seo
no flags Details | Formatted Diff | Diff
FontPlatformData (12.67 KB, patch)
2010-10-19 16:22 PDT, Kwang Yul Seo
krit: review-
Details | Formatted Diff | Diff
FontCache (6.73 KB, patch)
2010-10-19 17:41 PDT, Kwang Yul Seo
krit: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kwang Yul Seo 2010-06-07 18:55:37 PDT
Use skia as Brew MP's graphics backend.
Comment 1 Kwang Yul Seo 2010-08-27 15:47:23 PDT
Created attachment 65777 [details]
FontCustomPlatformData

Reuse chromium's FontCustomPlatformData.
Comment 2 Kwang Yul Seo 2010-08-27 15:55:40 PDT
CC'ing David Levin as it touches chromium.
Comment 3 Kwang Yul Seo 2010-09-06 14:32:40 PDT
Comment on attachment 65777 [details]
FontCustomPlatformData

I will submit patches for WebKit Brew MP after bug 39672 is resolved.
Comment 4 Kwang Yul Seo 2010-09-14 17:34:58 PDT
Created attachment 67622 [details]
FontCustomPlatformData

Add PLATFORM(BREWMP) guard to reuse the OS(LINUX) code.
Comment 5 Kwang Yul Seo 2010-09-14 17:35:53 PDT
CC'ing James here.
Comment 6 James Robinson 2010-09-14 18:52:56 PDT
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.
Comment 7 Kwang Yul Seo 2010-09-14 19:05:08 PDT
(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.
Comment 8 Kwang Yul Seo 2010-09-20 15:20:57 PDT
Created attachment 68144 [details]
Font

Port Font to Brew MP.
Comment 9 Kwang Yul Seo 2010-09-20 15:40:58 PDT
Created attachment 68149 [details]
Font

Fix COMPILE_ASSERT.
Comment 10 Kwang Yul Seo 2010-09-20 17:42:41 PDT
(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.
Comment 11 Kwang Yul Seo 2010-10-19 11:02:13 PDT
Created attachment 71186 [details]
FontCustomPlatformData

Update the patch because OS(FREEBSD) guard was added.
Comment 12 Kwang Yul Seo 2010-10-19 16:22:22 PDT
Created attachment 71219 [details]
FontPlatformData
Comment 13 Kwang Yul Seo 2010-10-19 17:41:13 PDT
Created attachment 71232 [details]
FontCache
Comment 14 Kwang Yul Seo 2010-10-19 17:48:49 PDT
(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 15 James Robinson 2010-10-21 20:20:41 PDT
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.
Comment 16 Kwang Yul Seo 2010-10-25 10:19:37 PDT
(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 17 James Robinson 2010-10-27 10:48:50 PDT
Comment on attachment 71186 [details]
FontCustomPlatformData

OK!
Comment 18 WebKit Commit Bot 2010-10-27 11:05:31 PDT
Comment on attachment 71186 [details]
FontCustomPlatformData

Clearing flags on attachment: 71186

Committed r70675: <http://trac.webkit.org/changeset/70675>
Comment 19 Holger Freyther 2010-12-24 02:33:33 PST
Comment on attachment 68149 [details]
Font

Is there anything that is specific to BREW? If not you should really share the implementation.
Comment 20 Holger Freyther 2010-12-24 02:33:52 PST
Comment on attachment 68149 [details]
Font

Is there anything that is specific to BREW? If not you should really share the implementation.
Comment 21 Holger Freyther 2010-12-24 02:34:17 PST
Comment on attachment 68149 [details]
Font

Is there anything that is specific to BREW? If not you should really share the implementation.
Comment 22 Dirk Schulze 2011-02-09 02:14:48 PST
Comment on attachment 68149 [details]
Font

I agree to Holger. Looks like you should share the code than copying it.
Comment 23 Dirk Schulze 2011-02-09 02:15:07 PST
Comment on attachment 71219 [details]
FontPlatformData

ditto.
Comment 24 Dirk Schulze 2011-02-09 02:15:29 PST
Comment on attachment 71232 [details]
FontCache

ditto.
Comment 25 Kwang Yul Seo 2012-07-26 05:20:09 PDT
Brew MP port is no longer maintained.