[PATCH] Android: Port eglretracer for Android

[Alexander Monakov]

On Tue, 23 Jul 2013, Juha-Pekka Heikkilä wrote:
> > 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?
> 
> If the intention is to go Waffle way for Android why would I go
> sideloading with different APIs?

I've asked the code to be removed because the way it introduces waffle_dl_* is
very inelegant and does not appear to add anything useful.  If there are
reasons you want Waffle's dynamic loading be used in Apitrace, please make
that a separate patch, but for retracing just the glws_waffle.cpp code should
be sufficient.

> > 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.
> 
> I feel this is a bit iffy, mostly because these paths have remained static
> since honeycomb (I think). Removing the paths here will add extra hurdle
> for compiling this for Android while the paths almost everyone would use
> here are the same. Then again, removing the paths here would make Nigel's
> suggested alternative way of compiling possible but then everyone need to
> give these paths every time.
> 
> Anyone compiling eglretracer from this in any case have to have
> possibility to compile Waffle for their device somehow somewhere thus I
> assume most people using this would have the Android source tree on their
> build machine.

Hm, fair enough.  The way I would handle this: I would create a
FindWaffle.cmake module to be placed in Apitrace's cmake directory (but useful
for other cmake projects using Waffle).  Apitrace's CMakeLists would simply
call 'find_package(Waffle)', and Android repo lookup code could go in the
module without looking entirely out-of-place in the main CMakeLists.

> > Can this code go into the constructor instead and the static method be
> > removed?
> 
> I prefer the two stage constructor so I don't need to start throwing
> exceptions. All waffle functions can fail so I take them the C-ish way.

But you don't have any recovery code, you just immediately terminate the
application with assert(0) when Waffle fails.  You can do that in the
constructor as well.

> > 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.
> 
> No offence taken :) The thing here is that..
> 
> waffle_make_current(dpy, NULL, NULL);
> 
> ..look really straightforward as if would just do..
> 
> eglMakeCurrent(dpy, NULL, NULL, NULL);
> 
> ..but it does more than that and is platform dependant. This is
> undocumented behaviour within Waffle and looking at the Waffle code I do
> think it should be underlined here is used something which was not
> promised.

Can you ask Chad to document that, because, honestly, I don't see how Waffle's
make_current(?, NULL, NULL) could possibly have some effect other than "unbind
current context from the current thread".

Thanks.
Alexander