[waffle] [PATCH] Enabling Waffle library for Android.

[Chad Versace]

On 06/26/2012 04:18 AM, Juha-Pekka Heikkilä wrote:> This quite big patch will
enable Waffle for Android ICS, it currently use
> Android make files. I tested it with the examples/gl_basic test which also
> has the Android make file now.
>
> Most of the stuff in the src/waffle/android folder is very similar to rest
> of Waffle, just mixed with Android's SurfaceFlinger negotiation.
>
> Patch in the attachment.
>
> /Juha-Pekka

Thanks for doing this Android work. I've combed through the patch, and it mostly
looks good. There is a little bug, a few tabs, and one design problem involving
global variables that will affect complex test cases.

+LOCAL_C_INCLUDES := $(LOCAL_PATH)/include/ \
+    $(LOCAL_PATH)/src/ \
+    bionic \
+    bionic/libstdc++/include \
+    external/gtest/include \
+    external/stlport/stlport \

Is gtest needed? I couldn't find any use of it.

+LOCAL_SRC_FILES:= \
+    src/waffle/core/wcore_tinfo.c \
+    src/waffle/core/wcore_config_attrs.c \
+    src/waffle/core/wcore_error.c \
+	src/waffle/api/api_priv.c \
+	src/waffle/api/waffle_attrib_list.c \
+	src/waffle/api/waffle_config.c \
+	src/waffle/api/waffle_context.c \
+	src/waffle/api/waffle_display.c \
+	src/waffle/api/waffle_enum.c \
+	src/waffle/api/waffle_error.c \
+	src/waffle/api/waffle_gl_misc.c \
+	src/waffle/api/waffle_init.c \
+	src/waffle/api/waffle_window.c \

^^^ Tabs here...

+LOCAL_SHARED_LIBRARIES := \
+    libEGL \
+    libdl \
+	libGLESv2 \
+	libutils \
+	libgui \

^^^ and here too. A quick grep shows that these were the only tabs in the
patch.

[snip]

+struct waffle_android_window {
+    struct waffle_android_display display;
+    EGLSurface egl_surface;
+};

The use case for waffle_android_window is that it should provide enough data
to the user waffle_window_get_native() in order to enable him to
gain access to all the Android data structures necessary to bypass Waffle and
interact with the native window directly. For example, it should provide
enough data to the user in order to allow him to manage window input.

Soon, Piglit will begin using waffle_window_get_native() to manage window
input.

If at first we don't put all the data in waffle_android_window that should
belong there, it's not a huge problem. Items can later be appended to the
struct without breaking ABI.

I don't know much about the Android API's, but I think at least ANativeWindow
belongs in waffle_android_window. More about this below.

[snip]

+static union waffle_native_display*
+android_display_get_native(struct wcore_display *wc_self)
+{
+    struct android_display *self = android_display(wc_self);
+    struct waffle_android_display *n_dpy;
+
+    n_dpy = wcore_malloc(sizeof(*n_dpy));
+    if (n_dpy == NULL)
+        return NULL;
+

Here, android_display_fill_native() should be called.

+    return (union waffle_native_display*) n_dpy;
+}

[snip]

diff --git a/src/waffle/android/android_surfaceflingerlink.cpp
b/src/waffle/android/android_surfaceflingerlink.cpp
new file mode 100644
index 0000000..3d1f7a6
--- /dev/null
+++ b/src/waffle/android/android_surfaceflingerlink.cpp

[snip]

+EGLDisplay waffle_egl_display(EGL_NO_DISPLAY);
+EGLSurface waffle_egl_surface(EGL_NO_SURFACE);
+
+sp<SurfaceComposerClient> waffle_composer_client;
+sp<SurfaceControl> waffle_surface_control;

This global state needs to be removed. By maintaining this state globally,
bugs will occur if the user creates multiple surfaces. Also, this will make
more difficult any effort to make Waffle thread-safe. (Currently, most Waffle
functions are thread-safe; only a few need fixing).

In order to remove the global state, the code in this file needs to be
refactored approximately like this:

    - Move `waffle_composer_client` to `android_display::composer_client`.

    - Move `waffle_surface_control` to `android_window::surface_control`.

    - Move the ANativeWindow, created by SetupSurface(), to
      `android_window::a_good_clear_name_for_the_ANativeWindow`.

    - Replace use of `waffle_egl_display` with `EGLDisplay
      android_display::android_display`. (Also, it's probably a good idea to
      rename android_display::android_display to android_display::egl, in
      order to be consistent with all the other EGL platforms).

    - Replace use of `waffle_egl_surface` with `android_window::egl`.

Also, there is no need to isolate the Android platform's C++ code into
a single file. If Android requires you to use C++, then don't hesitate to
change android_{display,window}.c into C++ files.  On a similar note,
interfacing with MacOS requires use of the Cocoa Objective-C API's, so I made
all the files in the cgl directory Objective-C files.

+bool
+setup_surfaceflinger_link()
+{
+    bool bRVal;
+    EGLint iRVal;
+
+    waffle_egl_display = eglGetDisplay(EGL_DEFAULT_DISPLAY);
+    iRVal = eglGetError();
+    if (iRVal != EGL_SUCCESS) {
+        return false;
+    }
+
+    iRVal = eglGetError();
+    if (waffle_egl_display == EGL_NO_DISPLAY) {
+        return false;
+    }
+
+    eglInitialize(waffle_egl_display, NULL, NULL);
+    iRVal = eglGetError();
+    if (iRVal != EGL_SUCCESS) {
+        return false;
+    }
+    return true;
+}

Waffle already has a function that does this, android_egl_initialize(),
declared by android_priv_egl.h. You should use it because emits nice error
messages when things fail.

Function setup_surfaceflinger_link, and related functions android_init_gl and
android_get_waffle_egl_display, should removed. Instead,
android_display_connect() should just call
android_egl_initialize(EGL_DEFAULT_DISPLAY). See wayalnd_display.c:105 for an
example.

Thanks,
Chad