<html><head><title>Samsung Enterprise Portal mySingle</title>
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type">
<style id="mysingle_style" type="text/css">P {
        MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 12pt
}
TD {
        MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 12pt
}
LI {
        MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 12pt
}
BODY {
        LINE-HEIGHT: 1.4; MARGIN: 10px; FONT-FAMILY: Arial, arial; FONT-SIZE: 12pt
}
</style>

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