Bug 40275 - [BREWMP] Port graphics backend
: [BREWMP] Port graphics backend
Status: RESOLVED WONTFIX
: WebKit
Platform
: 528+ (Nightly build)
: Other Other
: P2 Normal
Assigned To:
:
:
:
: 33564
  Show dependency treegraph
 
Reported: 2010-06-07 18:55 PST by
Modified: 2012-07-26 05:20 PST (History)


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


Note

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


Description From 2010-06-07 18:55:37 PST
Use skia as Brew MP's graphics backend.
------- Comment #1 From 2010-08-27 15:47:23 PST -------
Created an attachment (id=65777) [details]
FontCustomPlatformData

Reuse chromium's FontCustomPlatformData.
------- Comment #2 From 2010-08-27 15:55:40 PST -------
CC'ing David Levin as it touches chromium.
------- Comment #3 From 2010-09-06 14:32:40 PST -------
(From update of attachment 65777 [details])
I will submit patches for WebKit Brew MP after bug 39672 is resolved.
------- Comment #4 From 2010-09-14 17:34:58 PST -------
Created an attachment (id=67622) [details]
FontCustomPlatformData

Add PLATFORM(BREWMP) guard to reuse the OS(LINUX) code.
------- Comment #5 From 2010-09-14 17:35:53 PST -------
CC'ing James here.
------- Comment #6 From 2010-09-14 18:52:56 PST -------
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 From 2010-09-14 19:05:08 PST -------
(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 From 2010-09-20 15:20:57 PST -------
Created an attachment (id=68144) [details]
Patch

Port Font to Brew MP.
------- Comment #9 From 2010-09-20 15:40:58 PST -------
Created an attachment (id=68149) [details]
Font

Fix COMPILE_ASSERT.
------- Comment #10 From 2010-09-20 17:42:41 PST -------
(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 From 2010-10-19 11:02:13 PST -------
Created an attachment (id=71186) [details]
FontCustomPlatformData

Update the patch because OS(FREEBSD) guard was added.
------- Comment #12 From 2010-10-19 16:22:22 PST -------
Created an attachment (id=71219) [details]
FontPlatformData
------- Comment #13 From 2010-10-19 17:41:13 PST -------
Created an attachment (id=71232) [details]
FontCache
------- Comment #14 From 2010-10-19 17:48:49 PST -------
(In reply to comment #13)
> Created an attachment (id=71232) [details] [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 From 2010-10-21 20:20:41 PST -------
(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.
------- Comment #16 From 2010-10-25 10:19:37 PST -------
(In reply to comment #15)
> (From update of attachment 71186 [details] [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 From 2010-10-27 10:48:50 PST -------
(From update of attachment 71186 [details])
OK!
------- Comment #18 From 2010-10-27 11:05:31 PST -------
(From update of attachment 71186 [details])
Clearing flags on attachment: 71186

Committed r70675: <http://trac.webkit.org/changeset/70675>
------- Comment #19 From 2010-12-24 02:33:33 PST -------
(From update of attachment 68149 [details])
Is there anything that is specific to BREW? If not you should really share the implementation.
------- Comment #20 From 2010-12-24 02:33:52 PST -------
(From update of attachment 68149 [details])
Is there anything that is specific to BREW? If not you should really share the implementation.
------- Comment #21 From 2010-12-24 02:34:17 PST -------
(From update of attachment 68149 [details])
Is there anything that is specific to BREW? If not you should really share the implementation.
------- Comment #22 From 2011-02-09 02:14:48 PST -------
(From update of attachment 68149 [details])
I agree to Holger. Looks like you should share the code than copying it.
------- Comment #23 From 2011-02-09 02:15:07 PST -------
(From update of attachment 71219 [details])
ditto.
------- Comment #24 From 2011-02-09 02:15:29 PST -------
(From update of attachment 71232 [details])
ditto.
------- Comment #25 From 2012-07-26 05:20:09 PST -------
Brew MP port is no longer maintained.