Re: [PATCH] skia: update the source to build with the latest skia

Thank you Bryce for merging this patch.

Thanks and Best Regards,

N Ravi

------- Original Message -------

Sender : Bryce William Harrington<[email protected]> Engineer, Staff 1/SRA-Silicon Valley-Open Source/Samsung Electronics

Date : Jul 08, 2014 23:41 (GMT+05:30)

Title : Re: [cairo] [PATCH] skia: update the source to build with the latest skia

Thanks, LGTM as well. I verified a build. Pushed to master.

Bryce

On Mon, Jun 30, 2014 at 05:08:00PM +0530, RAVI NANJUNDAPPA wrote:
> Hi,
>
> Please see attached the patch after incorporating review comments from Uli.
> Thanking Uli for the comments.
> Please help me in reviewing this patch.
>
> Please see further below for views from my side.
>
> > -----Original Message-----
> > From: cairo [mailto:[email protected]] On Behalf Of RAVI
> > NANJUNDAPPA
> > Sent: Wednesday, June 25, 2014 5:01 PM
> > To: 'Uli Schlachter'; [email protected]
> > Subject: Re: [cairo] [PATCH] skia: update the source to build with the
> latest
> > skia
> >
> > Hi,
> >
> > Please my comments inline below.
> >
> >
> > > -----Original Message-----
> > > From: cairo [mailto:[email protected]] On Behalf Of Uli
> > > Schlachter
> > > Sent: Wednesday, June 25, 2014 1:29 PM
> > > To: [email protected]
> > > Subject: Re: [cairo] [PATCH] skia: update the source to build with the
> > latest
> > > skia
> > >
> > > Hi,
> > >
> > > since Bruce (:-P) wants me to complain, here are my complaints:
> > >
> > > Is it somehow possible to check the required minimum Skia version
> > > during configure? Mostly just for documentation purpose?
> >
> > AFAIK, the latest skia is still at 1.0.0 version (as mentioned in
> > include/core/SkTypes.h file skia source dir).
> > Yes, we can check for the minimum required Skia version during configure.
> > But there is a problem. When we build skia source (either Debug or Release
> > build) it generates only libskia.so and not libskia.so.1.0 or
> libskia.so.1.0.0. And
> > also we don't get the skia binaries in the pkg format (for ex: no
> libskia-
> > 1.0.0.deb/rpm for run-time pkg, or libskia-dev-1.0.0.deb/rpm for
> > development pkgs).
> > In such a case, checking for the skia version in the standard way (using
> > PKG_CHECK_MODULES) will be bit difficult.
> > Please share your opinion on this.
> >
> > >
> > > And Skia really doesn't have anything like a stable API? :-(
> > >
> >
> > Yes. In my experience, Skia APIs are changing very rapidly. Not sure of
> the
> > reason though.
> >
> > > On 23.06.2014 10:44, Ravi Nanjundappa wrote:
> > > > This fixes several build related issues for the skia backend which
> > > > is introduced due to skia source up-gradation.
> > > >
> > > > Signed-off-by: Ravi Nanjundappa
> > > > ---
> > > > configure.ac | 12 ++++----
> > > > src/skia/cairo-skia-context.cpp | 61
> > > ++++++++++++++++++++++++++++++---------
> > > > src/skia/cairo-skia-private.h | 4 +--
> > > > src/skia/cairo-skia-surface.cpp | 9 ++----
> > > > 4 files changed, 58 insertions(+), 28 deletions(-)
> > > >
> > > > diff --git a/configure.ac b/configure.ac index fdcb2dc..04479ff
> > > > 100644
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -249,16 +249,16 @@ CAIRO_ENABLE_SURFACE_BACKEND(skia, Skia,
> > > no, [
> > > > [directory to find compiled skia sources])],
> > > > [skia_DIR="$withval"],
> > > > [skia_DIR="`pwd`/../skia"])
> > > > - AC_ARG_WITH([skia-bulid],
> > > > - [AS_HELP_STRING([--with-skia-build=(Release|Debug)]
> > > > + AC_ARG_WITH([skia-build-type],
> > > > + [AS_HELP_STRING([--with-skia-build-type=(Release|Debug)]
> > > > [build of skia to link with, default is
> > Release])],
> > >
> > > Why is it necessary to rename the configure option to fix the build?
> > >
> > > > - [skia_BUILD="$withval"],
> > > > - [skia_BUILD="Release"])
> > > > + [skia_BUILD_TYPE="$withval"],
> > > > + [skia_BUILD_TYPE="Release"])
> > > > skia_NONPKGCONFIG_CFLAGS="-I$skia_DIR/include/config -
> > > I$skia_DIR/include/core -I$skia_DIR/include/effects"
> > > > - if test "x$skia_BUILD" = x"Release"; then
> > > > + if test "x$skia_BUILD_TYPE" = "xRelease"; then
> > > > skia_NONPKGCONFIG_CFLAGS="-DSK_RELEASE -
> > > DSK_CAN_USE_FLOAT $skia_NONPKGCONFIG_CFLAGS"
> > > > fi
> > > > - skia_NONPKGCONFIG_LIBS="--start-group
> > > $skia_DIR/out/$skia_BUILD/obj.target/gyp/libeffects.a
> > > $skia_DIR/out/$skia_BUILD/obj.target/gyp/libimages.a
> > > $skia_DIR/out/$skia_BUILD/obj.target/gyp/libutils.a
> > > $skia_DIR/out/$skia_BUILD/obj.target/gyp/libopts.a
> > > $skia_DIR/out/$skia_BUILD/obj.target/gyp/libcore.a -end-group"
> > > > + skia_NONPKGCONFIG_LIBS="-
> > > L$skia_DIR/out/$skia_BUILD_TYPE/lib.target/ -lskia -lstdc++"
> > >
> > > Urgh. The old options scare me, the new ones also do so a bit. But at
> > least
> > > this is better than what we had before.
> > >
> > > Although the new -lstdc++ sounds like a non-portable (gcc specific?)
> > > hack
> > to
> > > work around some deeper issues in how the skia backend is build.
> > > However, I'll just look the other way for this one....
> >
> > -lstdc++ is required for new and delete operators used in the skia
> > -lstdc++ source
> > files majorly.
> >
> > >
> > > > AC_SUBST(skia_DIR)
> > > > ])
> > > >
> > > > diff --git a/src/skia/cairo-skia-context.cpp
> > > > b/src/skia/cairo-skia-context.cpp index bbe5507..c2d9fba 100644
> > > > --- a/src/skia/cairo-skia-context.cpp
> > > > +++ b/src/skia/cairo-skia-context.cpp
> > > > @@ -53,6 +53,7 @@
> > > > #include "cairo-skia-private.h"
> > > > #include "cairo-surface-backend-private.h"
> > > >
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -236,14 +237,15 @@ surface_to_sk_bitmap (cairo_surface_t
> > > > *surface, SkBitmap& bitmap) {
> > > > cairo_image_surface_t *img = (cairo_image_surface_t *) surface;
> > > > SkBitmap::Config config;
> > > > + SkColorType colorType;
> > > > bool opaque;
> > > >
> > > > if (unlikely (! format_to_sk_config (img->format, config,
> opaque)))
> > > > return false;
> > > >
> > > > bitmap.reset ();
> > > > - bitmap.setConfig (config, img->width, img->height, img->stride);
> > > > - bitmap.setIsOpaque (opaque);
> > > > + colorType = SkBitmapConfigToColorType(config);
> > > > + bitmap.setInfo (SkImageInfo::Make(img->width, img->height,
> > > > + colorType, kPremul_SkAlphaType), img->stride);
> > > > bitmap.setPixels (img->data);
> > >
> > > Opaque becomes an unused variable? Is that really the right thing?
> > > Ok. I looked at the code. Nope, it's not. "opaque" is the only thing
> > > that
> > tells
> > > apart ARGB32 and RGB24, both have config SkBitmap::kARGB_8888_Config.
> > >
> >
> > I understand your point. I'll try to find a way to differentiate between
> > ARGB32 and RGB24 formats.
> >
>
> Modified the source code to consider the "opaque" value using setAlphaType()
> to differentiate between alpha and non-alpha formats.
>
> > > > return true;
> > > > @@ -330,9 +332,18 @@ source_to_sk_shader (cairo_skia_context_t *cr,
> > > > if (surface->type == CAIRO_SURFACE_TYPE_SKIA) {
> > > > cairo_skia_surface_t *esurf = (cairo_skia_surface_t *)
> > > > surface;
> > > >
> > > > - shader = SkShader::CreateBitmapShader (*esurf->bitmap,
> > > > + if(! _cairo_matrix_is_identity (&pattern->matrix))
> > > > + {
> > > > + SkMatrix localMatrix = matrix_inverse_to_sk
> > (pattern->matrix);
> > > > + shader = SkShader::CreateBitmapShader (*esurf->bitmap,
> > > > + extend_to_sk (pattern-
> > > >extend),
> > > > + extend_to_sk (pattern-
> > > >extend),
> > > > + &localMatrix);
> > > > + } else {
> > > > + shader = SkShader::CreateBitmapShader (*esurf->bitmap,
> > > > extend_to_sk (pattern-
> > > >extend),
> > > > extend_to_sk (pattern-
> > > >extend));
> > > > + }
> > >
> > > Does this really need the "if"? I would just always use the code with
> > > localMatric.
> > >
> > > (Also: It should be "if (! " instead of "if(! " and the indentation is
> > wrong
> > > (spaces instead of tabs and too many of them)
> >
> > Thank for pointing it out. I'll correct these and re-submit the patch
> again.
>
> Here we've followed the same method as used for pattern->type ==
> CAIRO_PATTERN_TYPE_LINEAR (code below this source snippet). So I feel this
> should be fine.
> Please feel free to comment on it.
>
> > >
> > > > } else {
> > > > SkBitmap bitmap;
> > > >
> > > > @@ -351,9 +362,18 @@ source_to_sk_shader (cairo_skia_context_t *cr,
> > > > if (unlikely (! surface_to_sk_bitmap (surface, bitmap)))
> > > > return NULL;
> > > >
> > > > - shader = SkShader::CreateBitmapShader (bitmap,
> > > > - extend_to_sk (pattern-
> > > >extend),
> > > > - extend_to_sk (pattern-
> > > >extend));
> > > > + if(! _cairo_matrix_is_identity (&pattern->matrix))
> > > > + {
> > > > + SkMatrix localMatrix = matrix_inverse_to_sk
> > (pattern->matrix);
> > > > + shader = SkShader::CreateBitmapShader (bitmap,
> > > > + extend_to_sk
> > (pattern->extend),
> > > > + extend_to_sk
> > (pattern->extend),
> > > > + &localMatrix);
> > > > + } else {
> > > > + shader = SkShader::CreateBitmapShader (bitmap,
> > > > + extend_to_sk
> > (pattern->extend),
> > > > + extend_to_sk
> > (pattern->extend));
> > > > + }
> > >
> > > Same as above
> > >
> > > > }
> > > > } else if (pattern->type == CAIRO_PATTERN_TYPE_LINEAR
> > > > /* || pattern->type == CAIRO_PATTERN_TYPE_RADIAL */) @@
> > > > -382,8 +402,17 @@ source_to_sk_shader (cairo_skia_context_t *cr,
> > > > SkFloatToScalar (linear->pd1.y));
> > > > points[1].set (SkFloatToScalar (linear->pd2.x),
> > > > SkFloatToScalar (linear->pd2.y));
> > > > - shader = SkGradientShader::CreateLinear (points, colors, pos,
> > > gradient->n_stops,
> > > > +
> > > > + if(! _cairo_matrix_is_identity (&pattern->matrix))
> > > > + {
> > > > + SkMatrix localMatrix = matrix_inverse_to_sk (pattern-
> > > >matrix);
> > > > + shader = SkGradientShader::CreateLinear (points, colors,
> > pos,
> > > gradient->n_stops,
> > > > + extend_to_sk (pattern-
> > > >extend),
> > > > + 0, &localMatrix);
> > > > + } else {
> > > > + shader = SkGradientShader::CreateLinear (points, colors,
> > > > +pos, gradient->n_stops,
> > > > extend_to_sk (pattern-
> > > >extend));
> > > > + }
> > >
> > > same
> > >
> > > > } else {
> > > > // XXX todo -- implement real radial shaders in Skia
> > > > }
> > > > @@ -394,8 +423,8 @@ source_to_sk_shader (cairo_skia_context_t *cr,
> > > > }
> > > > }
> > > >
> > > > - if (shader && ! _cairo_matrix_is_identity (&pattern->matrix))
> > > > - shader->setLocalMatrix (matrix_inverse_to_sk (pattern->matrix));
> > > > + //if (shader && ! _cairo_matrix_is_identity (&pattern->matrix))
> > > > + //shader->setLocalMatrix (matrix_inverse_to_sk (pattern->matrix));
> > >
> > >
> > > Why do you comment this out? Just delete it.
> >
> > Sure. I'll delete these lines and re-submit the patch again.
>
> Yep. I've taken care of this.
>
> >
> > >
> > > > return shader;
> > > > }
> > > > @@ -446,6 +475,7 @@ _cairo_skia_context_set_source (void
> > *abstract_cr,
> > > > cr->paint->setColor (color);
> > > > } else {
> > > > SkShader *shader = source_to_sk_shader (cr, source);
> > > > + SkPaint::FilterLevel fLevel = (SkPaint::FilterLevel)
> > > > +pattern_filter_to_sk (source);
> > >
> > > This casts a boolean to SkPaint::FilterLevel and...
> > >
> > > > if (shader == NULL) {
> > > > UNSUPPORTED;
> > > > return CAIRO_STATUS_SUCCESS;
> > > > @@ -454,7 +484,9 @@ _cairo_skia_context_set_source (void
> > *abstract_cr,
> > > > cr->paint->setShader (shader);
> > > > shader->unref ();
> > > >
> > > > - cr->paint->setFilterBitmap (pattern_filter_to_sk (source));
> > > > + cr->paint->setFilterLevel(fLevel ?
> > > > + (SkPaint::kLow_FilterLevel) :
> > > > + (SkPaint::kNone_FilterLevel));
> > > > }
> > >
> > > ...then uses that as a boolean again. Really?
> > >
> > > (Oh and: wrong indentation)
> > >
> >
> > Sure. I'll recheck these and submit the changes again.
>
> I've rectified this part of the source to avoid un-necessary conversions.
> Thanks for pointing out. I somehow missed it initially.
>
> >
> > > > /* XXX change notification */
> > > > @@ -496,7 +528,10 @@ _cairo_skia_context_set_source_surface (void
> > > *abstract_cr,
> > > > cr->paint->setShader (shader);
> > > > shader->unref ();
> > > >
> > > > - cr->paint->setFilterBitmap (true);
> > > > + cr->paint->setFilterLevel(true ?
> > > > + (SkPaint::kLow_FilterLevel) :
> > > > + (SkPaint::kNone_FilterLevel));
> > > > +
> > >
> > > Why?! Can you fix this up, please? (Oh and of course: wrong
> > > indentation)
> > >
> >
> > Sure. I'll recheck these and submit the changes again.
>
> Same as above.
>
> >
> > > > return CAIRO_STATUS_SUCCESS;
> > > > }
> > > > @@ -682,7 +717,7 @@ _cairo_skia_context_set_dash (void *abstract_cr,
> > > > intervals[i++] = SkFloatToScalar (dashes[j]);
> > > > } while (loop--);
> > > >
> > > > - SkDashPathEffect *dash = new SkDashPathEffect (intervals,
> > > num_dashes, SkFloatToScalar (offset));
> > > > + SkDashPathEffect *dash = SkDashPathEffect::Create (intervals,
> > > > + num_dashes, SkFloatToScalar (offset));
> > > >
> > > > cr->paint->setPathEffect (dash);
> > > > dash->unref ();
> > > > @@ -1264,7 +1299,7 @@ _cairo_skia_context_paint_with_alpha (void
> > > *abstract_cr,
> > > > if (CAIRO_ALPHA_IS_OPAQUE (alpha))
> > > > return _cairo_skia_context_paint (cr);
> > > >
> > > > - cr->paint->setAlpha(SkScalarRound(255*alpha));
> > > > + cr->paint->setAlpha(SkScalarRoundToInt(255*alpha));
> > > > status = _cairo_skia_context_paint (cr);
> > > > cr->paint->setAlpha(255);
> > > >
> > > > diff --git a/src/skia/cairo-skia-private.h
> > > > b/src/skia/cairo-skia-private.h index de3897b..f538b48 100644
> > > > --- a/src/skia/cairo-skia-private.h
> > > > +++ b/src/skia/cairo-skia-private.h
> > > > @@ -111,11 +111,9 @@ format_to_sk_config (cairo_format_t format,
> > > > case CAIRO_FORMAT_A8:
> > > > config = SkBitmap::kA8_Config;
> > > > break;
> > > > - case CAIRO_FORMAT_A1:
> > > > - config = SkBitmap::kA1_Config;
> > > > - break;
> > > > case CAIRO_FORMAT_RGB30:
> > > > case CAIRO_FORMAT_INVALID:
> > > > + case CAIRO_FORMAT_A1:
> > > > default:
> > > > return false;
> > > > }
> > >
> > > Did skia really drop A1 surfaces?!
> >
> > Yes. It has dropped A1 surfaces (as in the skia commit id : commit
> > 11f392ed531f05e8de6b6af6ae607f90aeb080c6)
> > >
> > > > diff --git a/src/skia/cairo-skia-surface.cpp
> > > > b/src/skia/cairo-skia-surface.cpp index bb785e1..682f3db 100644
> > > > --- a/src/skia/cairo-skia-surface.cpp
> > > > +++ b/src/skia/cairo-skia-surface.cpp
> > > > @@ -207,9 +207,6 @@ sk_config_to_pixman_format_code
> > > (SkBitmap::Config
> > > > config,
> > > >
> > > > case SkBitmap::kA8_Config:
> > > > return PIXMAN_a8;
> > > > -
> > > > - case SkBitmap::kA1_Config:
> > > > - return PIXMAN_a1;
> > > > case SkBitmap::kRGB_565_Config:
> > > > return PIXMAN_r5g6b5;
> > > > case SkBitmap::kARGB_4444_Config:
> > > > @@ -217,7 +214,6 @@ sk_config_to_pixman_format_code
> > > (SkBitmap::Config
> > > > config,
> > > >
> > > > case SkBitmap::kNo_Config:
> > > > case SkBitmap::kIndex8_Config:
> > > > - case SkBitmap::kRLE_Index8_Config:
> > > > case SkBitmap::kConfigCount:
> > > > default:
> > > > ASSERT_NOT_REACHED;
> > > > @@ -236,6 +232,7 @@ _cairo_skia_surface_create_internal
> > > (SkBitmap::Config config,
> > > > cairo_skia_surface_t *surface;
> > > > pixman_image_t *pixman_image;
> > > > pixman_format_code_t pixman_format;
> > > > + SkColorType colorType;
> > > >
> > > > surface = (cairo_skia_surface_t *) malloc (sizeof
> > (cairo_skia_surface_t));
> > > > if (unlikely (surface == NULL)) @@ -256,8 +253,8 @@
> > > > _cairo_skia_surface_create_internal
> > > (SkBitmap::Config config,
> > > > _cairo_image_surface_init (&surface->image, pixman_image,
> > > > pixman_format);
> > > >
> > > > surface->bitmap = new SkBitmap;
> > > > - surface->bitmap->setConfig (config, width, height, surface-
> > > >image.stride);
> > > > - surface->bitmap->setIsOpaque (opaque);
> > > > + colorType = SkBitmapConfigToColorType(config);
> > > > + surface->bitmap->setInfo (SkImageInfo::Make(width, height,
> > > > + colorType, kPremul_SkAlphaType), surface->image.stride);
> > > > surface->bitmap->setPixels (surface->image.data);
> > > >
> > > > surface->image.base.is_clear = data == NULL;
> > >
> > > Same as above: What happened to "opaque"? It's now unused and in my
> > > local git tree it is the only difference between RGB24 and ARGB32, so
> > > it seems kind of important to me. And no, AFAIK we cannot assume that
> > > ARGB32 surfaces have "the right" value in that unused byte.
> > >
> > > Note that I have no clue about skia and will not give my Reviewed-by
> > > to
> > any
> > > patches touching it. The above is just what I noticed from a quick look.
> >
> > Thanks for your comments. I'll try to look at these and re-submit the
> patch
> > again with relevant changes, wherever necessary.
> > I hope that should be fine.
>
> Yes. Taken care of the usage of "opaque" value to differentiate between
> alpha and non-alpha formats.
>
> >
> > >
> > > Cheers,
> > > Uli
> > > --
> > > "Some people are worth melting for." - Olaf
> > > --
> > > cairo mailing list
> > > [email protected]
> > > http://lists.cairographics.org/mailman/listinfo/cairo
> >
> > Thanks and Best Regards,
> > N Ravi
> >
> >
> > --
> > cairo mailing list
> > [email protected]
> > http://lists.cairographics.org/mailman/listinfo/cairo
>
> Thanks and Best Regards,
> N Ravi

> >From 9abf2d93e54933968e2ab87cbe415a7e61672b39 Mon Sep 17 00:00:00 2001
> From: Ravi Nanjundappa 
> Date: Mon, 30 Jun 2014 17:05:26 +0530
> Subject: [PATCH] skia: update the source to build with the latest skia
> 
> This fixes several build related issues for the skia backend
> which is introduced due to skia source up-gradation.
> 
> Signed-off-by: Ravi Nanjundappa 
> ---
>  configure.ac | 12 ++++----
>  src/skia/cairo-skia-context.cpp | 65 +++++++++++++++++++++++++++++----------
>  src/skia/cairo-skia-private.h | 4 +--
>  src/skia/cairo-skia-surface.cpp | 13 +++-----
>  4 files changed, 60 insertions(+), 34 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index fdcb2dc..04479ff 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -249,16 +249,16 @@ CAIRO_ENABLE_SURFACE_BACKEND(skia, Skia, no, [
>  [directory to find compiled skia sources])],
>  [skia_DIR="$withval"],
>  [skia_DIR="`pwd`/../skia"])
> - AC_ARG_WITH([skia-bulid],
> - [AS_HELP_STRING([--with-skia-build=(Release|Debug)]
> + AC_ARG_WITH([skia-build-type],
> + [AS_HELP_STRING([--with-skia-build-type=(Release|Debug)]
>  [build of skia to link with, default is Release])],
> - [skia_BUILD="$withval"],
> - [skia_BUILD="Release"])
> + [skia_BUILD_TYPE="$withval"],
> + [skia_BUILD_TYPE="Release"])
>  skia_NONPKGCONFIG_CFLAGS="-I$skia_DIR/include/config -I$skia_DIR/include/core -I$skia_DIR/include/effects"
> - if test "x$skia_BUILD" = x"Release"; then
> + if test "x$skia_BUILD_TYPE" = "xRelease"; then
>  skia_NONPKGCONFIG_CFLAGS="-DSK_RELEASE -DSK_CAN_USE_FLOAT $skia_NONPKGCONFIG_CFLAGS"
>  fi
> - skia_NONPKGCONFIG_LIBS="--start-group $skia_DIR/out/$skia_BUILD/obj.target/gyp/libeffects.a $skia_DIR/out/$skia_BUILD/obj.target/gyp/libimages.a $skia_DIR/out/$skia_BUILD/obj.target/gyp/libutils.a $skia_DIR/out/$skia_BUILD/obj.target/gyp/libopts.a $skia_DIR/out/$skia_BUILD/obj.target/gyp/libcore.a -end-group"
> + skia_NONPKGCONFIG_LIBS="-L$skia_DIR/out/$skia_BUILD_TYPE/lib.target/ -lskia -lstdc++"
>  AC_SUBST(skia_DIR)
>  ])
>  
> diff --git a/src/skia/cairo-skia-context.cpp b/src/skia/cairo-skia-context.cpp
> index bbe5507..9ffb8f6 100644
> --- a/src/skia/cairo-skia-context.cpp
> +++ b/src/skia/cairo-skia-context.cpp
> @@ -53,6 +53,7 @@
>  #include "cairo-skia-private.h"
>  #include "cairo-surface-backend-private.h"
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -236,16 +237,19 @@ surface_to_sk_bitmap (cairo_surface_t *surface, SkBitmap& bitmap)
>  {
>  cairo_image_surface_t *img = (cairo_image_surface_t *) surface;
>  SkBitmap::Config config;
> + SkColorType colorType;
>  bool opaque;
>  
>  if (unlikely (! format_to_sk_config (img->format, config, opaque)))
>  return false;
>  
>  bitmap.reset ();
> - bitmap.setConfig (config, img->width, img->height, img->stride);
> - bitmap.setIsOpaque (opaque);
> + bitmap.setAlphaType (opaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType);
> + colorType = SkBitmapConfigToColorType(config);
> + bitmap.setInfo (SkImageInfo::Make(img->width, img->height, colorType, kPremul_SkAlphaType), img->stride);
>  bitmap.setPixels (img->data);
>  
> +
>  return true;
>  }
>  
> @@ -330,9 +334,18 @@ source_to_sk_shader (cairo_skia_context_t *cr,
>  if (surface->type == CAIRO_SURFACE_TYPE_SKIA) {
>  cairo_skia_surface_t *esurf = (cairo_skia_surface_t *) surface;
>  
> - shader = SkShader::CreateBitmapShader (*esurf->bitmap,
> - extend_to_sk (pattern->extend),
> - extend_to_sk (pattern->extend));
> + if (! _cairo_matrix_is_identity (&pattern->matrix))
> + {
> + SkMatrix localMatrix = matrix_inverse_to_sk (pattern->matrix);
> + shader = SkShader::CreateBitmapShader (*esurf->bitmap,
> + extend_to_sk (pattern->extend),
> + extend_to_sk (pattern->extend),
> + &localMatrix);
> + } else {
> + shader = SkShader::CreateBitmapShader (*esurf->bitmap,
> + extend_to_sk (pattern->extend),
> + extend_to_sk (pattern->extend));
> + }
>  } else {
>  SkBitmap bitmap;
>  
> @@ -351,9 +364,18 @@ source_to_sk_shader (cairo_skia_context_t *cr,
>  if (unlikely (! surface_to_sk_bitmap (surface, bitmap)))
>  return NULL;
>  
> - shader = SkShader::CreateBitmapShader (bitmap,
> - extend_to_sk (pattern->extend),
> - extend_to_sk (pattern->extend));
> + if (! _cairo_matrix_is_identity (&pattern->matrix))
> + {
> + SkMatrix localMatrix = matrix_inverse_to_sk (pattern->matrix);
> + shader = SkShader::CreateBitmapShader (bitmap,
> + extend_to_sk (pattern->extend),
> + extend_to_sk (pattern->extend),
> + &localMatrix);
> + } else {
> + shader = SkShader::CreateBitmapShader (bitmap,
> + extend_to_sk (pattern->extend),
> + extend_to_sk (pattern->extend));
> + }
>  }
>  } else if (pattern->type == CAIRO_PATTERN_TYPE_LINEAR
>  /* || pattern->type == CAIRO_PATTERN_TYPE_RADIAL */)
> @@ -382,8 +404,17 @@ source_to_sk_shader (cairo_skia_context_t *cr,
>  SkFloatToScalar (linear->pd1.y));
>  points[1].set (SkFloatToScalar (linear->pd2.x),
>  SkFloatToScalar (linear->pd2.y));
> - shader = SkGradientShader::CreateLinear (points, colors, pos, gradient->n_stops,
> - extend_to_sk (pattern->extend));
> +
> + if(! _cairo_matrix_is_identity (&pattern->matrix))
> + {
> + SkMatrix localMatrix = matrix_inverse_to_sk (pattern->matrix);
> + shader = SkGradientShader::CreateLinear (points, colors, pos, gradient->n_stops,
> + extend_to_sk (pattern->extend),
> + 0, &localMatrix);
> + } else {
> + shader = SkGradientShader::CreateLinear (points, colors, pos, gradient->n_stops,
> + extend_to_sk (pattern->extend));
> + }
>  } else {
>  // XXX todo -- implement real radial shaders in Skia
>  }
> @@ -394,9 +425,6 @@ source_to_sk_shader (cairo_skia_context_t *cr,
>  }
>  }
>  
> - if (shader && ! _cairo_matrix_is_identity (&pattern->matrix))
> - shader->setLocalMatrix (matrix_inverse_to_sk (pattern->matrix));
> -
>  return shader;
>  }
>  
> @@ -446,6 +474,7 @@ _cairo_skia_context_set_source (void *abstract_cr,
>  cr->paint->setColor (color);
>  } else {
>  SkShader *shader = source_to_sk_shader (cr, source);
> + bool fLevel = pattern_filter_to_sk (source);
>  if (shader == NULL) {
>  UNSUPPORTED;
>  return CAIRO_STATUS_SUCCESS;
> @@ -454,7 +483,8 @@ _cairo_skia_context_set_source (void *abstract_cr,
>  cr->paint->setShader (shader);
>  shader->unref ();
>  
> - cr->paint->setFilterBitmap (pattern_filter_to_sk (source));
> + cr->paint->setFilterLevel (fLevel ?
> + (SkPaint::kLow_FilterLevel) : (SkPaint::kNone_FilterLevel));
>  }
>  
>  /* XXX change notification */
> @@ -496,7 +526,8 @@ _cairo_skia_context_set_source_surface (void *abstract_cr,
>  cr->paint->setShader (shader);
>  shader->unref ();
>  
> - cr->paint->setFilterBitmap (true);
> + cr->paint->setFilterLevel (true ?
> + (SkPaint::kLow_FilterLevel) : (SkPaint::kNone_FilterLevel));
>  
>  return CAIRO_STATUS_SUCCESS;
>  }
> @@ -682,7 +713,7 @@ _cairo_skia_context_set_dash (void *abstract_cr,
>  intervals[i++] = SkFloatToScalar (dashes[j]);
>  } while (loop--);
>  
> - SkDashPathEffect *dash = new SkDashPathEffect (intervals, num_dashes, SkFloatToScalar (offset));
> + SkDashPathEffect *dash = SkDashPathEffect::Create (intervals, num_dashes, SkFloatToScalar (offset));
>  
>  cr->paint->setPathEffect (dash);
>  dash->unref ();
> @@ -1264,7 +1295,7 @@ _cairo_skia_context_paint_with_alpha (void *abstract_cr,
>  if (CAIRO_ALPHA_IS_OPAQUE (alpha))
>  return _cairo_skia_context_paint (cr);
>  
> - cr->paint->setAlpha(SkScalarRound(255*alpha));
> + cr->paint->setAlpha(SkScalarRoundToInt(255*alpha));
>  status = _cairo_skia_context_paint (cr);
>  cr->paint->setAlpha(255);
>  
> diff --git a/src/skia/cairo-skia-private.h b/src/skia/cairo-skia-private.h
> index de3897b..f538b48 100644
> --- a/src/skia/cairo-skia-private.h
> +++ b/src/skia/cairo-skia-private.h
> @@ -111,11 +111,9 @@ format_to_sk_config (cairo_format_t format,
>  case CAIRO_FORMAT_A8:
>  config = SkBitmap::kA8_Config;
>  break;
> - case CAIRO_FORMAT_A1:
> - config = SkBitmap::kA1_Config;
> - break;
>  case CAIRO_FORMAT_RGB30:
>  case CAIRO_FORMAT_INVALID:
> + case CAIRO_FORMAT_A1:
>  default:
>  return false;
>  }
> diff --git a/src/skia/cairo-skia-surface.cpp b/src/skia/cairo-skia-surface.cpp
> index bb785e1..9b16bd2 100644
> --- a/src/skia/cairo-skia-surface.cpp
> +++ b/src/skia/cairo-skia-surface.cpp
> @@ -64,7 +64,7 @@ _cairo_skia_surface_create_similar (void *asurface,
>  
>  if (content == surface->image.base.content)
>  {
> - config = surface->bitmap->getConfig ();
> + config = surface->bitmap->config ();
>  opaque = surface->bitmap->isOpaque ();
>  }
>  else if (! format_to_sk_config (_cairo_format_from_content (content),
> @@ -207,9 +207,6 @@ sk_config_to_pixman_format_code (SkBitmap::Config config,
>  
>  case SkBitmap::kA8_Config:
>  return PIXMAN_a8;
> -
> - case SkBitmap::kA1_Config:
> - return PIXMAN_a1;
>  case SkBitmap::kRGB_565_Config:
>  return PIXMAN_r5g6b5;
>  case SkBitmap::kARGB_4444_Config:
> @@ -217,8 +214,6 @@ sk_config_to_pixman_format_code (SkBitmap::Config config,
>  
>  case SkBitmap::kNo_Config:
>  case SkBitmap::kIndex8_Config:
> - case SkBitmap::kRLE_Index8_Config:
> - case SkBitmap::kConfigCount:
>  default:
>  ASSERT_NOT_REACHED;
>  return (pixman_format_code_t) -1;
> @@ -236,6 +231,7 @@ _cairo_skia_surface_create_internal (SkBitmap::Config config,
>  cairo_skia_surface_t *surface;
>  pixman_image_t *pixman_image;
>  pixman_format_code_t pixman_format;
> + SkColorType colorType;
>  
>  surface = (cairo_skia_surface_t *) malloc (sizeof (cairo_skia_surface_t));
>  if (unlikely (surface == NULL))
> @@ -256,8 +252,9 @@ _cairo_skia_surface_create_internal (SkBitmap::Config config,
>  _cairo_image_surface_init (&surface->image, pixman_image, pixman_format);
>  
>  surface->bitmap = new SkBitmap;
> - surface->bitmap->setConfig (config, width, height, surface->image.stride);
> - surface->bitmap->setIsOpaque (opaque);
> + colorType = SkBitmapConfigToColorType(config);
> + surface->bitmap->setAlphaType (opaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType);
> + surface->bitmap->setInfo (SkImageInfo::Make(width, height, colorType, kPremul_SkAlphaType), surface->image.stride);
>  surface->bitmap->setPixels (surface->image.data);
>  
>  surface->image.base.is_clear = data == NULL;
> -- 
> 1.7.9.5
> 

> --
> cairo mailing list
> [email protected]
> http://lists.cairographics.org/mailman/listinfo/cairo

-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo