[PATCH] Android: Port eglretracer for Android
Alexander Monakov
amonakov at ispras.ru
Tue Jul 23 05:10:15 PDT 2013
On Tue, 23 Jul 2013, Juha-Pekka Heikkila wrote:
> diff --git a/dispatch/glproc_egl.cpp b/dispatch/glproc_egl.cpp
> index 5f93b14..f9e4ac1 100644
> --- a/dispatch/glproc_egl.cpp
> +++ b/dispatch/glproc_egl.cpp
> @@ -26,6 +26,10 @@
>
> #include "glproc.hpp"
>
> +#if defined __ANDROID__ && defined BUILDEGLRETRACER
> +#include <waffle.h>
> +#endif
> +
>
> #if !defined(_WIN32)
> #ifndef _GNU_SOURCE
> @@ -137,6 +141,10 @@ _getPublicProcAddress(const char *procName)
> void *
> _getPrivateProcAddress(const char *procName)
> {
> +#if defined __ANDROID__ && defined BUILDEGLRETRACER
> + extern int waffle_gl_api;
> + return waffle_dl_sym(waffle_gl_api, procName);
> +#else
Why do you need Waffle to handle symbol lookups for Apitrace? As far as I
understand, you only need the surface+context handling from Waffle, and
Apitrace's own dynamic loading code should be sufficient. Can you drop all
code that references waffle_gl_api from this patch?
Furthermore, I think it's wrong that you patch _getPrivateProcAddress but not
_getPublicProcAddress.
> diff --git a/retrace/CMakeLists.txt b/retrace/CMakeLists.txt
> index 913c23e..e77d549 100644
> --- a/retrace/CMakeLists.txt
> +++ b/retrace/CMakeLists.txt
> @@ -39,6 +39,33 @@ add_library (retrace_common STATIC
> retrace_swizzle.cpp
> json.cpp
> )
> +
> +if (ANDROID)
> + message ("-- Looking for Waffle library")
> +
> +#########
> +# In following the name waffle-1 should be fixed to found out properly.
> + find_path (Waffle_INCLUDE_DIRS
> + NAMES waffle.h
> + PATHS "$ENV{OUT}/obj/include/waffle-1"
> + "$ENV{OUT}/obj/lib" "$ENV{ANDROID_BUILD_TOP}/external/waffle"
> + NO_CMAKE_FIND_ROOT_PATH
> + )
> + if (Waffle_INCLUDE_DIRS)
> + message ("-- Looking for Waffle library - found")
> + find_library (Waffle
> + NAMES "waffle-1"
> + PATHS "$ENV{OUT}/obj/include/waffle-1" "$ENV{OUT}/obj/lib"
> + "$ENV{ANDROID_BUILD_TOP}/external/waffle"
> + REQUIRED NO_CMAKE_FIND_ROOT_PATH NO_DEFAULT_PATH
> + )
> + include_directories (${Waffle_INCLUDE_DIRS})
> + else ()
> + message ("-- Looking for Waffle library - not found,"
> + " skipping eglretrace")
> + endif ()
> +endif ()
> +
I would suggest not introducing knowledge of Android repo layout into
CMakeLists, and instead make it the responsibility of the user to make sure
that the (cross-)compiler can find Waffle's include files and libraries. That
can be arranged with environment variables such as LIBRARY_PATH and CPATH.
> target_link_libraries (retrace_common
> image
> common
> @@ -112,24 +139,45 @@ if (WIN32 OR APPLE OR X11_FOUND)
> install (TARGETS glretrace RUNTIME DESTINATION bin)
> endif ()
>
> -if (ENABLE_EGL AND X11_FOUND AND NOT WIN32 AND NOT APPLE)
> - add_executable (eglretrace
> - glws_egl_xlib.cpp
> - )
> +
> +if (ENABLE_EGL AND X11_FOUND OR (ANDROID AND Waffle_INCLUDE_DIRS)AND NOT WIN32
> + AND NOT APPLE)
> + if (ANDROID)
> + add_definitions(-DBUILDEGLRETRACER)
> + add_executable (eglretrace
> + glws_waffle.cpp
> + )
> + else ()
> + add_executable (eglretrace
> + glws_egl_xlib.cpp
> + )
> + endif ()
Please keep the old "if" statement as is and add a new one for glws_waffle.cpp.
>
> add_dependencies (eglretrace glproc)
>
> - target_link_libraries (eglretrace
> - retrace_common
> - glretrace_common
> - glproc_egl
> - ${X11_X11_LIB}
> - ${CMAKE_THREAD_LIBS_INIT}
> - dl
> - )
> + if (ANDROID)
> + target_link_libraries (eglretrace
> + retrace_common
> + glretrace_common
> + glproc_egl
> + dl
> + ${Waffle}
> + )
> + else ()
> + target_link_libraries (eglretrace
> + retrace_common
> + glretrace_common
> + glproc_egl
> + ${X11_X11_LIB}
> + ${CMAKE_THREAD_LIBS_INIT}
> + dl
> + )
> + endif ()
>
> if (${CMAKE_SYSTEM_NAME} MATCHES "Linux")
> - target_link_libraries (eglretrace rt)
> + if (NOT ANDROID)
> + target_link_libraries (eglretrace rt)
> + endif ()
> if (READPROC_H_FOUND)
> target_link_libraries (eglretrace ${proc_LIBRARY})
> endif ()
> diff --git a/retrace/glws_waffle.cpp b/retrace/glws_waffle.cpp
> new file mode 100644
> index 0000000..f0c44ab
> --- /dev/null
> +++ b/retrace/glws_waffle.cpp
> @@ -0,0 +1,274 @@
> +#include "os.hpp"
> +#include "waffle.h"
> +#include "glws.hpp"
> +
> +extern "C" {
> +int waffle_gl_api = 0;
> +};
> +
> +namespace glws {
> +
> +struct waffle_display *dpy;
> +
> +class WaffleVisual : public Visual
> +{
> +public:
> + struct waffle_config *config;
> +
> + WaffleVisual() :
> + config(NULL)
> + {}
> +
> + static WaffleVisual* createWaffleVisual(bool doubleBuffer, Profile profile)
> + {
Can this code go into the constructor instead and the static method be
removed?
> + int config_attrib_list[64], i(0);
> + WaffleVisual *visual = new WaffleVisual();
> +
> + config_attrib_list[i++] = WAFFLE_CONTEXT_API;
> +
> + switch (profile) {
> + case PROFILE_COMPAT:
> + if(!waffle_display_supports_context_api(dpy, WAFFLE_DL_OPENGL))
> + {
> + os::log("%s: !waffle_display_supports_context_api\n",
> + __FILE__);
> +
> + delete visual;
> + assert(0);
> + return NULL;
> + }
> +
> + waffle_gl_api = config_attrib_list[i++] = WAFFLE_DL_OPENGL;
WAFFLE_DL_x enum cannot be used in config_attrib_list.
> + break;
> + case PROFILE_CORE:
> + os::log("%s: PROFILE_CORE no support\n", __FILE__);
> + delete visual;
> + assert(0);
> + return NULL;
> + case PROFILE_ES1:
> + if(!waffle_display_supports_context_api(dpy,
> + WAFFLE_CONTEXT_OPENGL_ES1))
> + {
> + os::log("%s: !waffle_display_supports_context_api\n",
> + __FILE__);
> +
> + delete visual;
> + assert(0);
> + return NULL;
> + }
> +
> + config_attrib_list[i++] = WAFFLE_CONTEXT_OPENGL_ES1;
> + waffle_gl_api = WAFFLE_DL_OPENGL_ES1;
> + break;
> + case PROFILE_ES2:
> + if(!waffle_display_supports_context_api(dpy,
> + WAFFLE_CONTEXT_OPENGL_ES2))
> + {
> + os::log("%s: !waffle_display_supports_context_api\n",
> + __FILE__);
> +
> + delete visual;
> + assert(0);
> + return NULL;
> + }
> +
> + config_attrib_list[i++] = WAFFLE_CONTEXT_OPENGL_ES2;
> + waffle_gl_api = WAFFLE_DL_OPENGL_ES2;
> + break;
> + default:
> + os::log("%s: Unknown context profile\n", __FILE__);
> + delete visual;
> + assert(0);
> + return NULL;
> + }
> +
> + config_attrib_list[i++] = WAFFLE_RED_SIZE;
> + config_attrib_list[i++] = 8;
> + config_attrib_list[i++] = WAFFLE_GREEN_SIZE;
> + config_attrib_list[i++] = 8;
> + config_attrib_list[i++] = WAFFLE_BLUE_SIZE;
> + config_attrib_list[i++] = 8;
> + config_attrib_list[i++] = WAFFLE_DEPTH_SIZE;
> + config_attrib_list[i++] = 8;
> + config_attrib_list[i++] = WAFFLE_ALPHA_SIZE;
> + config_attrib_list[i++] = 8;
> + config_attrib_list[i++] = WAFFLE_STENCIL_SIZE;
> + config_attrib_list[i++] = 8;
> + config_attrib_list[i++] = WAFFLE_DOUBLE_BUFFERED;
> + config_attrib_list[i++] = doubleBuffer;
> + config_attrib_list[i++] = 0;
> +
> + visual->config = waffle_config_choose(dpy, config_attrib_list);
> + if (!visual->config)
> + {
> + os::log("Error in %s waffle_config_choose(dpy, " \
> + "config_attrib_list)\n", __FILE__);
> + delete visual;
> + assert(0);
> + return NULL;
> + }
> + return visual;
> + }
> +
> + ~WaffleVisual() {
> + if (this->config != NULL)
> + {
> + waffle_config_destroy(this->config);
> + this->config = NULL;
> + }
> + }
> +};
> +
> +class WaffleDrawable : public Drawable
> +{
> +public:
> +
> + struct waffle_window *window;
> +
> + WaffleDrawable(const Visual *vis, int w, int h, bool pbuffer) :
> + Drawable(vis, w, h, pbuffer)
> + {
> + const WaffleVisual *waffleVisual =
> + static_cast<const WaffleVisual *>(vis);
> +
> + window = waffle_window_create(waffleVisual->config, w, h);
> + }
> +
> + void
> + resize(int w, int h) {
> + if (w == width && h == height) {
> + return;
> + }
> +
> + waffle_window_resize(window, w, h);
> + Drawable::resize(w, h);
> + }
> +
> + void show(void) {
> + if (visible) {
> + return;
> + }
> +
> + waffle_window_show(window);
> + Drawable::show();
> + }
> +
> + void swapBuffers(void) {
> + waffle_window_swap_buffers(window);
> + }
> +};
> +
> +class WaffleContext : public Context
> +{
> +public:
> + struct waffle_context *context;
> +
> + WaffleContext(const Visual *vis, Profile prof) :
> + Context(vis, prof),
> + context(NULL)
> + {}
> +
> + static WaffleContext* createWaffleContext(const Visual *vis, Profile prof)
> + {
Likewise: can this go into the constructor?
> + WaffleContext *ctx = new WaffleContext(vis, prof);
> + const WaffleVisual *waffleVisual =
> + static_cast<const WaffleVisual *>(vis);
> +
> + ctx->context = waffle_context_create(waffleVisual->config, NULL);
> + if (!ctx->context)
> + {
> + os::log("Error in %s waffle_context_create(config, NULL)\n",
> + __FILE__);
> +
> + assert(0);
> + return NULL;
> + }
> + return ctx;
> + }
> +
> + ~WaffleContext() {
> + if( context != NULL )
> + {
> + waffle_context_destroy(context);
> + context = NULL;
> + }
> + }
> +};
> +
> +/*
> + With waffle there is not too many events to look for..
> + */
> +bool
> +processEvents(void) {
> + return true;
> +}
> +
> +void
> +init(void) {
> + int i;
> + int waffle_init_attrib_list[3];
> +
> + i = 0;
> + waffle_init_attrib_list[i++] = WAFFLE_PLATFORM;
> + waffle_init_attrib_list[i++] = WAFFLE_PLATFORM_ANDROID;
> + waffle_init_attrib_list[i++] = WAFFLE_NONE;
> +
> + waffle_init(waffle_init_attrib_list);
> +
> + dpy = waffle_display_connect(NULL);
> + if (!dpy)
> + {
> + os::log("No display!!\n");
> + assert(0);
> + }
> +}
> +
> +void
> +cleanup(void) {
> + waffle_display_disconnect(dpy);
> +}
> +
> +Visual *
> +createVisual(bool doubleBuffer, Profile profile) {
> + return WaffleVisual::createWaffleVisual(doubleBuffer, profile);
> +}
> +
> +Drawable *
> +createDrawable(const Visual *visual, int width, int height, bool pbuffer)
> +{
> + return new WaffleDrawable(visual, width, height, pbuffer);
> +}
> +
> +Context *
> +createContext(const Visual *visual, Context *shareContext, Profile profile,
> + bool debug)
> +{
> + return WaffleContext::createWaffleContext(visual, profile );
> +}
> +
> +bool
> +makeCurrent(Drawable *drawable, Context *context)
> +{
> + if (!drawable || !context) {
> + /*
> + Following cause inside waffle
> + eglMakeCurrent(dpy, NULL, NULL, NULL) which equal to
> + eglMakeCurrent(eglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, \
> + EGL_NO_CONTEXT)
> + Lets hope things stay that way...
> + */
The comment basically says that Waffle's make_current(NULL, NULL) has a
totally reasonable and expected behavior. Combined with the lack of code
comments in the rest of the file, this creates a somewhat comical effect, at
least to me. :) No offence meant, just pointing out the imbalance.
> + return waffle_make_current(dpy, NULL, NULL);
> + }
> + else {
} else {
Thanks.
Alexander
More information about the apitrace
mailing list