Re: create simple skia app for android using jni (issue 16336004)

[djso...@google.com]

move this code into platform_tools/android/ndk_demo_app. Also don't
include the
libskia_android.so in the CL, but rather put a README file with
instructions on
how to build this using just the NDK tools.


https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/buildme.sh
File experimental/HelloSkia/buildme.sh (right):

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/buildme.sh#newcode1
experimental/HelloSkia/buildme.sh:1: NDK_DEBUG=1 ndk-build && ant debug
&& adb install -r bin/HelloSkia-debug.apk
remove this file or clean it up and document what each step is supposed
to do.

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/Android.mk
File experimental/HelloSkia/jni/Android.mk (right):

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/Android.mk#newcode3
experimental/HelloSkia/jni/Android.mk:3: # libskia
add comments here about what this code snippet does.

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/Android.mk#newcode6
experimental/HelloSkia/jni/Android.mk:6: LOCAL_MODULE := skia_local
rename to skia_android

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/Android.mk#newcode16
experimental/HelloSkia/jni/Android.mk:16: LOCAL_MODULE := HelloSkia
rename to hello_skia_ndk.

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/Android.mk#newcode21
experimental/HelloSkia/jni/Android.mk:21: LOCAL_LDLIBS := -ljnigraphics
comment on why this is needed.

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/Android.mk#newcode24
experimental/HelloSkia/jni/Android.mk:24:
/usr/local/google/home/zachr/src/skia_so/trunk/include/core
give the user instructions on where they are supposed to get these
include files. Should they copy them into their jni directory under
skia/includes/[core/config/...]?

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/helloskia.cpp
File experimental/HelloSkia/jni/helloskia.cpp (right):

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/helloskia.cpp#newcode16
experimental/HelloSkia/jni/helloskia.cpp:16: JNIEXPORT void JNICALL
Java_com_example_HelloSkiaActivity_drawIntoBitmap(JNIEnv* env,
add a brief comment explaining what this method does.

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/helloskia.cpp#newcode18
experimental/HelloSkia/jni/helloskia.cpp:18: {
move to previous line

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/helloskia.cpp#newcode27
experimental/HelloSkia/jni/helloskia.cpp:27: SkString
path("skhello.png");
remove this unless you plan on drawing it.

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/helloskia.cpp#newcode28
experimental/HelloSkia/jni/helloskia.cpp:28: SkString text("Skia is the
Best!");
define text where you use it not at the beginning of the function.

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/helloskia.cpp#newcode31
experimental/HelloSkia/jni/helloskia.cpp:31: SkPaint paint;
same as above (define it closer to where it is used).

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/helloskia.cpp#newcode44
experimental/HelloSkia/jni/helloskia.cpp:44: // Draw something
"interesting"
let's draw some more interesting stuff. You can probably get some ideas
from looking at our sample slides. Also put a comment next to each
interesting group of drawing ops describing what is drawn.

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/helloskia.cpp#newcode52
experimental/HelloSkia/jni/helloskia.cpp:52: SkBitmap canvasBitmap;
you shouldn't need this at all.

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/jni/helloskia.cpp#newcode57
experimental/HelloSkia/jni/helloskia.cpp:57: for (uint32_t row = 0; row
< dstInfo.height; row++)
this isn't very efficient. We should just have our canvas write directly
into. We should just create the SkSurface using NewRasterDirect(...).

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/res/layout/main.xml
File experimental/HelloSkia/res/layout/main.xml (right):

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/res/layout/main.xml#newcode10
experimental/HelloSkia/res/layout/main.xml:10: android:text="Hello
World, HelloSkiaActivity"
why do you need a text view? Can't we draw it all through Skia?

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/src/com/example/HelloSkiaActivity.java
File experimental/HelloSkia/src/com/example/HelloSkiaActivity.java
(right):

https://codereview.chromium.org/16336004/diff/1/experimental/HelloSkia/src/com/example/HelloSkiaActivity.java#newcode44
experimental/HelloSkia/src/com/example/HelloSkiaActivity.java:44: Bitmap
skiaBitmap = Bitmap.createBitmap(400, 400, Bitmap.Config.ARGB_8888);
make the bitmap the same size as the canvas you are given.

https://codereview.chromium.org/16336004/

[djso...@google.com]

It is getting pretty close. I would add a README file to this directory
that
has instructions for...

1) how to build the skia lib and copy the necessary files into thei jni
folder
(e.g. run android_make -d ...)

2) how to run ndk-build

3) how to build the app

4) how to deploy the app


https://codereview.chromium.org/16336004/diff/9001/experimental/HelloSkia/jni/Android.mk
File experimental/HelloSkia/jni/Android.mk (right):

https://codereview.chromium.org/16336004/diff/9001/experimental/HelloSkia/jni/Android.mk#newcode3
experimental/HelloSkia/jni/Android.mk:3: # set this to wherever your
skia directory is located, with out trailing slashes
I think that for clarity we should have them copy the headers into
jni/skia/include. That way if they update their skia tree it is clear
that they would also need to update these files.

Additionally, have them put the shared library in jni/skia/ as well.

Now this file won't be dependent upon any variables being set, just that
they did the work ahead of time to put content into the correct
directories.

https://codereview.chromium.org/16336004/diff/9001/experimental/HelloSkia/jni/helloskia.cpp
File experimental/HelloSkia/jni/helloskia.cpp (right):

https://codereview.chromium.org/16336004/diff/9001/experimental/HelloSkia/jni/helloskia.cpp#newcode13
experimental/HelloSkia/jni/helloskia.cpp:13: static float get_seconds()
doesn't look like you call this function

https://codereview.chromium.org/16336004/diff/9001/experimental/HelloSkia/jni/helloskia.cpp#newcode44
experimental/HelloSkia/jni/helloskia.cpp:44:
keep the basic text drawing as well, but I like your animation idea!
That is interesting. :)

https://codereview.chromium.org/16336004/

[djso...@google.com]

the code looks good to me. The only thing that needs to be done is to move
this
out of experimental and into platform_tools/android/ndk_demo_app

https://codereview.chromium.org/16336004/

[djso...@google.com]

how about we create a new folder called examples/ and put it in there under
hello_skia_app

https://codereview.chromium.org/16336004/

[djso...@google.com]

this on is my fault, sorry. I intended it to be
platform_tools/android/examples.

https://codereview.chromium.org/16336004/

[djso...@google.com]

lgtm



https://codereview.chromium.org/16336004/

[djso...@google.com]

now that you have an lgtm you can click the commit button and it should go
the
the skia commit queue (CQ).

https://codereview.chromium.org/16336004/

[djso...@google.com]

lgtm with the removed images. I'll upload them in the morning.

https://codereview.chromium.org/16336004/

[oanh le]

Hi all,

I try to make a simple skia app by follow https://chromiumcodereview.appspot.com/16336004.

and i have a error: 

jni/skia/include/core/SkUserConfig.h:179:28: fatal error: utils/misc.h: No such file or directory 

I used Skia android 6.0

How to fix this problem?