Last Comment Bug 751431 - [Skia] Rip out Skia from libxul
: [Skia] Rip out Skia from libxul
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: mozilla15
Assigned To: Nick Cameron [:nrc]
:
: Gregory Szorc [:gps]
Mentors:
Depends on: 751463 752380
Blocks: 687187 746883
  Show dependency treegraph
 
Reported: 2012-05-02 17:21 PDT by Nick Cameron [:nrc]
Modified: 2012-05-23 04:50 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 1: configure/make changes (2.67 KB, patch)
2012-05-02 20:26 PDT, Nick Cameron [:nrc]
no flags Details | Diff | Splinter Review
patch 2: Skia API changes (3.92 KB, patch)
2012-05-02 20:26 PDT, Nick Cameron [:nrc]
no flags Details | Diff | Splinter Review
patch (14.68 KB, patch)
2012-05-03 21:15 PDT, Nick Cameron [:nrc]
no flags Details | Diff | Splinter Review
patch (15.33 KB, patch)
2012-05-06 16:52 PDT, Nick Cameron [:nrc]
no flags Details | Diff | Splinter Review
patch - without Freetype stuff (1.51 KB, patch)
2012-05-06 17:15 PDT, Nick Cameron [:nrc]
khuey: review+
Details | Diff | Splinter Review

Description Nick Cameron [:nrc] 2012-05-02 17:21:57 PDT

    
Comment 1 Nick Cameron [:nrc] 2012-05-02 20:26:21 PDT
Created attachment 620571 [details] [diff] [review]
patch 1: configure/make changes
Comment 2 Nick Cameron [:nrc] 2012-05-02 20:26:53 PDT
Created attachment 620572 [details] [diff] [review]
patch 2: Skia API changes
Comment 3 :Ehsan Akhgari 2012-05-02 20:34:40 PDT
What are SKIA_DLL and SKIA_IMPLEMENTATION?
Comment 4 Nick Cameron [:nrc] 2012-05-02 20:59:45 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #3)
> What are SKIA_DLL and SKIA_IMPLEMENTATION?

SKIA_DLL indicates that Skia is being built as a dll and SKIA_IMPLEMENTATION indicates that we are building Skia itself, and not a program which uses Skia (obviously we are doing both). The effect of setting SKIA_DLL and SKIA_IMPLEMENTATION is that SK_API is set to __declspec(dllexport), so the Skia classes are exported by the DLL.
Comment 5 Nick Cameron [:nrc] 2012-05-02 21:00:48 PDT
These patches will change quite a lot when Bas lands his patch to rip out Azure.
Comment 6 Mozilla RelEng Bot 2012-05-03 08:45:22 PDT
Try run for 59b97b2bb57d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=59b97b2bb57d
Results (out of 296 total builds):
    exception: 1
    success: 259
    warnings: 29
    failure: 2
    other: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ncameron@mozilla.com-59b97b2bb57d
 Timed out after 12 hours without completing.
Comment 7 Mozilla RelEng Bot 2012-05-03 09:00:23 PDT
Try run for 59b97b2bb57d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=59b97b2bb57d
Results (out of 296 total builds):
    exception: 1
    success: 260
    warnings: 29
    failure: 2
    other: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ncameron@mozilla.com-59b97b2bb57d
 Timed out after 12 hours without completing.
Comment 8 Mozilla RelEng Bot 2012-05-03 09:30:23 PDT
Try run for 59b97b2bb57d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=59b97b2bb57d
Results (out of 296 total builds):
    exception: 1
    success: 263
    warnings: 30
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ncameron@mozilla.com-59b97b2bb57d
 Timed out after 12 hours without completing.
Comment 9 Nick Cameron [:nrc] 2012-05-03 21:15:20 PDT
Created attachment 620948 [details] [diff] [review]
patch

New version, sits on top of Bas's patch in bug 751463.

I'm nowhere near convinced this is the best solution, or execution of it, but it might just work. Hopefully, bug 738014 will be a long term solution.
Comment 10 Nick Cameron [:nrc] 2012-05-03 21:16:18 PDT
Bas: I'll get khuey to review this once you like it (and it works, waiting on a tryserver push now).
Comment 11 Nick Cameron [:nrc] 2012-05-04 04:48:57 PDT
The patch doesn't work as advertised and I seem to have hit the limit of my configure/make knowledge with this one. I am doing this in configure.in:

MOZ_ENABLE_FREETYPE=1
AC_DEFINE(MOZ_ENABLE_FREETYPE)
AC_SUBST(MOZ_ENABLE_FREETYPE)

And in gfx/2d/Makefile.in I have:

ifdef MOZ_ENABLE_FREETYPE
CPPSRCS	+= \
        ScaledFontFreetype.cpp \
        $(NULL)
endif

But ScaledFontFreetype.cpp does not get compiled. If I remove the ifdef, then it compiles fine. I can't find anything to do in configure.in that sets MOZ_ENABLE_FREETYPE.

What am I missing?
Comment 12 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-05-04 05:10:34 PDT
I think you need to add to config/autoconf.mk.in
Comment 13 Nick Cameron [:nrc] 2012-05-06 16:52:47 PDT
Created attachment 621477 [details] [diff] [review]
patch
Comment 14 Bas Schouten (:bas.schouten) 2012-05-06 17:04:50 PDT
If possible please deal with the freetype stuff in a separate patch/bug first if possible, and then proceed with the move out of libxul.
Comment 15 Nick Cameron [:nrc] 2012-05-06 17:15:40 PDT
Created attachment 621481 [details] [diff] [review]
patch - without Freetype stuff
Comment 16 Nick Cameron [:nrc] 2012-05-07 17:31:19 PDT
Try run: https://tbpl.mozilla.org/?tree=Try&rev=b2b3dbd3193d
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-05-22 19:26:16 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c7cc13ba63a
Comment 18 Ed Morley [:emorley] 2012-05-23 04:50:50 PDT
https://hg.mozilla.org/mozilla-central/rev/6c7cc13ba63a

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