We should import Skia into our source tree and get it building so we can use it as an Azure backend. Not sure who I should be getting to review this, and what else I need to do regarding license issues etc. The cairo makefile change shouldn't be in here, but we fail to link on mac without (and fail to link everywhere else with it). For some reason this patch is stopping the cairo deflate code from being able to link with zlib. It's looking for the original symbols, and we only export prefixed ones (like _MOZ_Z_deflateInit) from our copy of zlib. I really have no idea how this patch could affect this :(

This was too big to attach to the bugzilla as a patch file. This patch just imports all the files we need into gfx/skia.

So it seems unlikely that we'll be able to import Skia without patching it. I'd like the update process to be better than we have with cairo. The third-party libraries we have in media/ are good example of the way to do this. i.e the README_MOZILLA and update.sh scripts

Added update.sh/README_MOZILLA file for easier updating of skia source code

Added android build support

(In reply to Matt Woodrow (:mattwoodrow) from comment #5) > Created attachment 562337 [details] [diff] [review] [diff] [details] [review] > Build system changes for Skia v2 > > Added android build support It seems like the shell script could do better than a long list of files?

(In reply to Jeff Muizelaar [:jrmuizel] from comment #6) > (In reply to Matt Woodrow (:mattwoodrow) from comment #5) > > Created attachment 562337 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review] > > Build system changes for Skia v2 > > > > Added android build support > > It seems like the shell script could do better than a long list of files? Also, I wouldn't bother uploading the actual skia parts of the patch because I won't be reviewing them.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #6) > > It seems like the shell script could do better than a long list of files? This is what the media/ script files were doing, what would you suggest instead? Copy the whole folders?

(In reply to Matt Woodrow (:mattwoodrow) from comment #8) > (In reply to Jeff Muizelaar [:jrmuizel] from comment #6) > > > > It seems like the shell script could do better than a long list of files? > > This is what the media/ script files were doing, what would you suggest > instead? I think the number of files in the media scripts is a lot more manageable and they likely need to copy some files and leave others. > Copy the whole folders? Doing that and using globs would do the job.

Not sure if you want to review this Jeff, seems like a good start though.

Comment on attachment 567641 [details] [diff] [review] Skia Makefile Review of attachment 567641 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine, a few nits. ::: gfx/Makefile.in @@ +55,5 @@ > +endif > + > +ifeq (android,$(MOZ_WIDGET_TOOLKIT)) > +DIRS += skia > +endif Collapse these two conditions to ifeq (,$(filter-out cocoa android,$(MOZ_WIDGET_TOOLKIT))) ::: gfx/skia/Makefile.in @@ +48,5 @@ > +EXPORT_LIBRARY = 1 > + > +EXPORTS_NAMESPACES = skia > + > +DEFINES += -DSK_A32_SHIFT=24 -DSK_R32_SHIFT=16 -DSK_G32_SHIFT=8 -DSK_B32_SHIFT=0 Should these really all be hardcoded here? @@ +52,5 @@ > +DEFINES += -DSK_A32_SHIFT=24 -DSK_R32_SHIFT=16 -DSK_G32_SHIFT=8 -DSK_B32_SHIFT=0 > + > +LOCAL_INCLUDES += -I$(srcdir)/include/core -I$(srcdir)/include/config -I$(srcdir)/include/ports -I$(srcdir)/src/core -I$(srcdir)/include/images -I$(srcdir)/include/utils/mac -I$(srcdir)/include/views -I$(srcdir)/include/effects > + > +VPATH += $(srcdir)/src/core $(srcdir)/src/ports $(srcdir)/src/opts $(srcdir)/src/effects Use multiple lines for these please. e.g. FOO = \ BAR \ BAZ \ $(NULL) @@ +55,5 @@ > + > +VPATH += $(srcdir)/src/core $(srcdir)/src/ports $(srcdir)/src/opts $(srcdir)/src/effects > + > +EXPORTS_skia = \ > + include/core/Sk64.h \ If you're VPATHing all of this above, you shouldn't need to say include/core on all of these, right? @@ +60,5 @@ > + include/core/SkAutoKern.h \ > + include/core/SkBitmap.h \ > + include/core/SkBlitRow.h \ > + include/core/SkBlitter.h \ > + include/core/SkBounder.h \ Consistent indentation please. ::: toolkit/library/libxul-config.mk @@ +330,5 @@ > $(MOZ_APP_EXTRA_LIBS) \ > $(SQLITE_LIBS) \ > $(NULL) > > + Don't add this extra line. @@ +365,5 @@ > +endif > + > +ifeq ($(MOZ_WIDGET_TOOLKIT),android) > +EXTRA_DSO_LDOPTS += $(MOZ_SKIA_LIBS) > +endif Same about the collapsed conditional.

Thanks Kyle! Gerv: Is there anything I need to do wrt licenses etc before landing this?

Skia is under the Apache License 2.0, according to its Wikipedia page. Is that correct? We will need to update about:license to account for this. We don't have any other Apache-licensed code yet, but recently when we made the decision to move to MPL 2, that made it possible to take Apache-2-licensed code. Once you have done the import, can you file a mozilla.org/Licensing bug on me to update about:licence? Gerv

(In reply to Gervase Markham [:gerv] from comment #15) > Skia is under the Apache License 2.0, according to its Wikipedia page. Is > that correct? Nope. It has recently been changed to a BSD style license. See: http://code.google.com/p/skia/source/browse/trunk/LICENSE

OK :-) Still, we need to update about:licence. If you could produce a patch for me to review (it should be obvious what to do - just make sure they are in alphabetical order) that would be awesome. Gerv

Comment on attachment 569899 [details] [diff] [review] License Changes Looks great, thanks :-) Gerv

https://hg.mozilla.org/integration/mozilla-inbound/rev/99297e426f81 https://hg.mozilla.org/integration/mozilla-inbound/rev/ebed635187a2 https://hg.mozilla.org/integration/mozilla-inbound/rev/d2fce6b656e1

https://hg.mozilla.org/mozilla-central/rev/656a231abf23 https://hg.mozilla.org/mozilla-central/rev/99297e426f81 https://hg.mozilla.org/mozilla-central/rev/ebed635187a2 https://hg.mozilla.org/mozilla-central/rev/d2fce6b656e1

The bug summary says mac although my Linux gcc 4.3.2 build breaks because of trailing commas in various header files (core/SkAdvancedTypefaceMetrics.h, core/SkCanvas.h, core/SkDevice.h, core/SkFlattenable.h, core/SkMaskFilter.h, core/SkPaint.h, core/SkTypes.h, effects/SkLayerDrawLooper.h)...