[Skia] Rip out Skia from libxul

RESOLVED FIXED in mozilla15

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nrc, Assigned: nrc)

Tracking

unspecified
mozilla15
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Comment hidden (empty)
(Assignee)

Updated

5 years ago
Blocks: 687187
(Assignee)

Comment 1

5 years ago
Created attachment 620571 [details] [diff] [review]
patch 1: configure/make changes
(Assignee)

Comment 2

5 years ago
Created attachment 620572 [details] [diff] [review]
patch 2: Skia API changes
What are SKIA_DLL and SKIA_IMPLEMENTATION?
(Assignee)

Comment 4

5 years ago
(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.
(Assignee)

Comment 5

5 years ago
These patches will change quite a lot when Bas lands his patch to rip out Azure.
(Assignee)

Updated

5 years ago
Depends on: 751463

Comment 6

5 years ago
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

5 years ago
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

5 years ago
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.
(Assignee)

Comment 9

5 years ago
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.
Attachment #620571 - Attachment is obsolete: true
Attachment #620572 - Attachment is obsolete: true
Attachment #620948 - Flags: review?(bas.schouten)
(Assignee)

Comment 10

5 years ago
Bas: I'll get khuey to review this once you like it (and it works, waiting on a tryserver push now).
(Assignee)

Comment 11

5 years ago
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?
(Assignee)

Updated

5 years ago
Attachment #620948 - Flags: review?(bas.schouten)
I think you need to add to config/autoconf.mk.in
(Assignee)

Comment 13

5 years ago
Created attachment 621477 [details] [diff] [review]
patch
Attachment #620948 - Attachment is obsolete: true
Attachment #621477 - Flags: review?(bas.schouten)
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.
(Assignee)

Updated

5 years ago
Depends on: 752380
(Assignee)

Comment 15

5 years ago
Created attachment 621481 [details] [diff] [review]
patch - without Freetype stuff
Attachment #621477 - Attachment is obsolete: true
Attachment #621477 - Flags: review?(bas.schouten)
Attachment #621481 - Flags: review?(khuey)
(Assignee)

Comment 16

5 years ago
Try run: https://tbpl.mozilla.org/?tree=Try&rev=b2b3dbd3193d
Attachment #621481 - Flags: review?(khuey) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c7cc13ba63a
https://hg.mozilla.org/mozilla-central/rev/6c7cc13ba63a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.