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

[Chad Versace]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/31/2012 05:16 AM, Juha-Pekka Heikkilä wrote:
> Here is now reworked version of the big patch, again in the attachment. 
> Tabs should be now gone :)

Thanks for killing the tabs :)

After reviewing the patch, I see three problems, discussed below. Let's get
those three squashed and push the patch.

> I mostly managed to follow your suggestions. I did stick with the structure
> most of the files being .c files with just one .cpp file, reason for this
> was "DEFINE_CONTAINER_CAST_FUNC" define which did not play nice with cpp
> and changing this define would cause me to change too many places which I
> cannot test and probably break them.

I'm ok with the files remaining .c files. Whichever choice keeps the code
working and simple is the better choice.

> waffle_android_window now has a pointer called pSFContainer that point to 
> android_surfaceflinger_container structure defined in 
> android_surfaceflingerlink.cpp. This structure contain Android related 
> classes. This way of providing access to these classes is again because of 
> the mixture of c and cpp.
> 
> The idea you mentioned piglit soon also accepting window input is going to 
> need some extra work on Android side. Currently I have the created 
> surface(s) floating on top of normal Android UI where Android will receive 
> all input and my surfaces input-wise behave as if they were not there.

I think floating surfaces will work well enough for now.

Problem 1:

I'd prefer that Waffle not expose a user-facing interface on Android that is
known not to work and will hence later need to be changed. Since your surfaces
do not repond to input, and the main reason for exposing the types in
waffle_android.h is to enable Waffle clients to implement input, then I think
the current best approach for the waffle_X_get_native functions is to
  1. Delete waffle_android.h.
  2. Let each waffle_X_get_native return null and emit WAFFLE_ERROR_UNSUPPORTED.

For an example of how that should look, see commit 1f52c43.


Problem 2:

If the user calls waffle_window_create() twice, then eglCreateWindowSurface
gets called twice on the same ANativeWindow. What should happen is that two
distinct windows be created.

The underlying problem is that the Android window objects, SurfaceControl and
ANativeWindow, should be created during waffle_window_create and be contained
in the android_window, not during waffle_display_create and in the
android_display.

(I do not understand what SurfaceComposerClient does, so I am unsure if it
should belong to android_display or android_window. I suspect android_display).


Problem 3:

This is a small problem that causes no bugs. Static functions shouldn't be
declared in headers unless they are also inline. That is, the following
function declarations need to be removed:

  android_config.h:android_config_destroy
  android_context.h:android_context_destroy
  android_display.h:android_display_disconnect
  android_display.h:android_display_supports_context_api
  android_window.h:android_window_destroy
  android_window.h:android_window_show
  android_window.h:android_window_swap_buffers

In android_context.c, I see that you define android_context_create above
android_context_destroy. That's ok, but the build will fail after removing the
declaration at android_context.h:android_context_destroy. To fix the build
without changing the order of things in android_context.c, you will need to
move the declaration at android_context.h:android_context_destroy to the top
of android_context.c. The same applies to android_config_destroy,
android_display_disconnect, and android_window_destroy.

- ----

Apologies for taking 7 days to respond to your patch. I'll try to be quicker
next time. If you have any objections or questions to this review, just let me
know.

- ----
Chad Versace
chad.versace at linux.intel.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJQIaX0AAoJEAIvNt057x8iTcIP/2eq6/IH9pG55EAMykXAvaFW
rQn0yuf/TtzA8FDl7c/M7ZWU0WN1fbapQA11Qb/zkaN0Y/pjHgFiPcKSjINozRDd
4fZKswcEizqtOynzyl2wkd11hfFr8XoO6bUQlEzYqRlnqXVJh/KCVbNXyRSgyaOw
ginm1/mtboGnrYkHJgtV1uQnRJSIGokXqN2+vrZCTwxlRbiY8CKloMqvaqCm9mWd
jZB4MWB0T1pXPsF6PLLyw7tsyuj5eC/Gj+nhfLsujQsBshwI3ptN/4wKjmbZ+yCP
blV16Zx5BnaTBBxTuXpqgHB++oEwgO7DRfCj+FRQ8tutQz8deTnLYDmuwocb6Lm9
ILS4qJ4yhQqW+Crax4a5W6pmw4fMz50n526ZPdCnuWsVqe8KHog7znrALqsJMEbS
oUvhRSrBRX64VNxzxk5Z1MfYod/sjwzNXNmFKAvmd5Aeilz+NlHk2Ao+nTOp78fF
F+7SQ4M4VcukAZZxnFiulAwUrPo8FQIv6eaBNjFEGniFx43ZViBQ+nxlsAXAPT2S
JGnP20qIKMN1W/DzkI/eayDjC9+6ADsYLzckq/3Z52+OcI9v5XhBy0ku1UbDxOLt
lPCnW8r7e0YBOLvFS6ZicfLoo+2L3z4IfchqxP3VZoXaEGqC1vDJ4D60MWxcx4FO
2Emjo/iypsQVTGBUfhw0
=rRPZ
-----END PGP SIGNATURE-----