Re: [waffle] [PATCH 10/12] egl: implement platform-specific information

[Frank Henigman]

On Fri, Jan 8, 2016 at 7:50 AM, Emil Velikov  wrote:
> On 6 January 2016 at 21:56, Frank Henigman  wrote:
>> Implement the platform hook of waffle_display_info_json() so it can
>> pick up egl-specific information.
>>
>> Signed-off-by: Frank Henigman 
>> ---
>>  src/waffle/egl/wegl_display.c  | 32 ++--
>>  src/waffle/egl/wegl_display.h  |  4 
>>  src/waffle/egl/wegl_platform.h |  3 +++
>>  3 files changed, 37 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/waffle/egl/wegl_display.c b/src/waffle/egl/wegl_display.c
>> index 88fce7a..dcfe934 100644
>> --- a/src/waffle/egl/wegl_display.c
>> +++ b/src/waffle/egl/wegl_display.c
>> @@ -25,6 +25,8 @@
>>
>>  #include 
>>
>> +#include "json.h"
>> +
>>  #include "wcore_error.h"
>>  #include "wcore_platform.h"
>>
>> @@ -63,7 +65,6 @@ wegl_display_init(struct wegl_display *dpy,
>>  {
>>  struct wegl_platform *plat = wegl_platform(wc_plat);
>>  bool ok;
>> -EGLint major, minor;
>>
>>  ok = wcore_display_init(>wcore, wc_plat);
>>  if (!ok)
>> @@ -75,7 +76,7 @@ wegl_display_init(struct wegl_display *dpy,
>>  goto fail;
>>  }
>>
>> -ok = plat->eglInitialize(dpy->egl, , );
>> +ok = plat->eglInitialize(dpy->egl, >major, >minor);
>>  if (!ok) {
>>  wegl_emit_error(plat, "eglInitialize");
>>  goto fail;
>> @@ -139,3 +140,30 @@ wegl_display_supports_context_api(struct wcore_display 
>> *wc_dpy,
>>
>>  return wc_plat->vtbl->dl_can_open(wc_plat, waffle_dl);
>>  }
>> +
>> +void
>> +wegl_display_info_json(struct wcore_display *wc_dpy, struct json *jj)
>> +{
>> +struct wegl_display *dpy = wegl_display(wc_dpy);
>> +struct wegl_platform *plat = wegl_platform(dpy->wcore.platform);
>> +
>> +const char *version = plat->eglQueryString(dpy->egl, EGL_VERSION);
>> +const char *vendor = plat->eglQueryString(dpy->egl, EGL_VENDOR);
>> +#ifdef EGL_VERSION_1_2
>> +const char *apis = plat->eglQueryString(dpy->egl, EGL_CLIENT_APIS);
>> +#endif
> I would suggesting adding the define on top ifndef EGL_VERSION_1_2,
> and dropping the checks here and below.

Sorry, not sure what you mean by this.  It sounds like skipping the
whole thing ifndef EGL_VERSION_1_2 ?  And that's ok because no one
uses <= 1.2 anymore?
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 07/12] waffle: support platform-specific information

[Frank Henigman]

On Fri, Jan 8, 2016 at 7:44 AM, Emil Velikov  wrote:
> On 6 January 2016 at 21:56, Frank Henigman  wrote:
>> Add a platform hook so platform-specific information can be included
>> by waffle_display_info_json().
>>
>> Signed-off-by: Frank Henigman 
>> ---
>>  src/waffle/api/waffle_display.c  | 10 +-
>>  src/waffle/core/wcore_platform.h |  4 
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/waffle/api/waffle_display.c 
>> b/src/waffle/api/waffle_display.c
>> index 7abe2ef..d6988ac 100644
>> --- a/src/waffle/api/waffle_display.c
>> +++ b/src/waffle/api/waffle_display.c
>> @@ -367,8 +367,16 @@ waffle_display_info_json(struct waffle_display *self, 
>> bool platform_too)
>>
>>  json_appendv(jj, "{", "generic", "{", "");
>>  add_generic_info(jj, wc_self->current_context);
>> -json_appendv(jj, "}", "}", "");
>> +json_append(jj, "}");
>>
>> +if (platform_too) {
>> +json_appendv(jj, "platform", "{", "");
>> +if (api_platform->vtbl->display.info_json)
>> +api_platform->vtbl->display.info_json(wc_self, jj);
> The rest of waffle tends to set UNSUPPORTED_ON_PLATFORM if the backend
> is missing the vfunc.

I'm reluctant to set an error for something that isn't clearly an
error (it might just be that a backend doesn't have platform-specific
details) because it could add a burden to the user to distinguish this
case from definite errors.  If someone does need to act on the
presence or absence or platform-specifics, they can always examine the
json.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 06/12] wflinfo: add option for JSON output

[Frank Henigman]

On Fri, Apr 8, 2016 at 8:14 PM, Chad Versace  wrote:
> On 01/06/2016 11:56 AM, Frank Henigman wrote:
>> With "-f json" wflinfo will now print the result of
>> waffle_display_info_json().
>>
>> Signed-off-by: Frank Henigman 
>> ---
>>  src/utils/wflinfo.c | 40 
>>  1 file changed, 36 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/utils/wflinfo.c b/src/utils/wflinfo.c
>> index 268d4b8..58f5688 100644
>> --- a/src/utils/wflinfo.c
>> +++ b/src/utils/wflinfo.c
>
>
>> @@ -1119,9 +1137,23 @@ main(int argc, char **argv)
>>  glGetStringi = waffle_get_proc_address("glGetStringi");
>>  }
>>
>> -ok = print_wflinfo();
>> -if (!ok)
>> -error_waffle();
>> +char *info;
>> +switch (opts.format) {
>> +case FORMAT_ORIGINAL:
>> +ok = print_wflinfo();
>> +if (!ok)
>> +error_waffle();
>> +break;
>> +
>> +case FORMAT_JSON:
>> +info = waffle_display_info_json(dpy, false);
>> +if (info) {
>> +printf("%s\n", info);
>> +free(info);
>> +} else
>> +error_waffle();
>> +break;
>> +}
>
>
> I agree with the patch series's goals: teach wflinfo to print 
> platform-specific
> info (such as EGL info); extract that info inside libwaffle and expose it
> through public API; that public API should provide JSON, and perhaps 
> additional
> formats.
>
> But this patch 6/12 poses a problem. Post-patch, wflinfo can print its info in
> the old format and in a json format. But, depending on the output format,
> wflinfo uses completely separate code paths to obtain the information.
>
> The actual information that wflinfo provides, and the method by which it
> obtains that info, should be largely independent of the output format. When 
> the
> user chooses an output format, that choice should affect the *presentation* 
> but
> not fundamentally affect the *content*.
>
> So...
>
> For wflinfo's sake, we need a public waffle_get_my_info() function that 
> returns
> data that can be translated into the old format and into the json format, or 
> perhaps
> returns the old format and json directly.
>
> To move forward with this patch series, I see the following options:
>
> 1. waffle_get_my_info() returns only json
>
>   I don't like this. This would require that wflinfo decode the json in
>   order to provide the old format output. I would like to avoid decoding
>   json in Waffle though, because that would require either (a) Waffle
>   contain a json decoder implementation or (b) Waffle rely on some json
>   library. As for (a), I don't want to maintain a json-decoder (a json
>   *encoder*, though, I don't object to). As for (b), Waffle is such
>   a small, focused project that it feels wrong to pull in a json-decoder
>   library as a dependency.
>
>   BUT, maybe this option could work and I'm overestimating the maintenance
>   overhead of decoding json.
>
> 2. waffle_get_my_info() returns only some-other-well-defined-format-FOO
>
>I don't like this either. Just like option 1, it would require that
>wflinfo decode the FOO format in order to provide json output.
>
> 3. waffle_get_my_info() can return a json string or a string in the old
>wflinfo format
>
>If option 3 can be implemented cleanly, I think it's the best choice.
>This option eliminates the need for decoding any special format.
>
>Perhaps we could implement this by defining a private struct...
>
> struct wcore_display_info {
> struct {
> char *platform;
> char *api;
> } waffle;
>
> struct {
> char *vendor;
> char *renderer;
> ...
> char **extensions;
> } opengl;
>
> struct {
> char *version;
> char *vendor;
> char *client_apis;
> ...
> } egl;
>
> ...
> };
>
> ...that waffle_get_my_info() populates and then encodes into the
> respected format: JSON, the old wflinfo format, a format that
> mimics glxinfo, or whatever.
>
>
> Frank, what do you think?

I really hope we can keep this simple.

I mentioned in the cover letter to this series that the next step is
to gut wflinfo and translate json to the old format.  We can insert
that after this patch if you like.  Option 1 is the nicest by far, if
the translator is written in python.  It's just a few lines of code to
read the json, plus a table that lists the correspondence between old
format lines and json keys.  So we have some really simple C code to
call the waffle api and get the json, and a really simple 

Re: [waffle] [PATCH 05/12] waffle: add waffle_display_info_json()

[Frank Henigman]

On Fri, Jan 8, 2016 at 7:40 AM, Emil Velikov  wrote:
> On 6 January 2016 at 21:56, Frank Henigman  wrote:
>> Duplicate wflinfo functionality in the API, with the difference that the
>> information is returned in JSON form.
>> The function has a parameter for including platform-specific information,
>> but it is ignored for now.
>>
>> Signed-off-by: Frank Henigman 
>> ---
>>  include/waffle/waffle.h |   5 +
>>  man/waffle_display.3.xml|  19 +++
>>  src/waffle/api/waffle_display.c | 284 
>> +++-
>>  src/waffle/waffle.def.in|   1 +
>>  4 files changed, 308 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/waffle/waffle.h b/include/waffle/waffle.h
>> index df0218e..1800399 100644
>> --- a/include/waffle/waffle.h
>> +++ b/include/waffle/waffle.h
>> @@ -214,6 +214,11 @@ bool
>>  waffle_display_supports_context_api(struct waffle_display *self,
>>  int32_t context_api);
>>
>> +#if WAFFLE_API_VERSION >= 0x0106
>> +char*
>> +waffle_display_info_json(struct waffle_display *self, bool platform_too);
> The function does not work solely with the display, but it requires a
> (bound) context. Thus it does not really fit waffle naming scheme. I'm
> afraid that I'm short of suggestions though (barring my "returning
> json formatted data sounds iffy, lets use tokens" rant from earlier)
>
>
>> +#endif
>> +
>>  union waffle_native_display*
>>  waffle_display_get_native(struct waffle_display *self);
>>
>> diff --git a/man/waffle_display.3.xml b/man/waffle_display.3.xml
>> index 9896247..5358472 100644
>> --- a/man/waffle_display.3.xml
>> +++ b/man/waffle_display.3.xml
>> @@ -24,6 +24,7 @@
>>  waffle_display
>>  waffle_display_connect
>>  waffle_display_disconnect
>> +waffle_display_info_json
>>  waffle_display_supports_context_api
>>  waffle_display_get_native
>>  class waffle_display
>> @@ -58,6 +59,12 @@ struct waffle_display;
>>
>>
>>
>> +char* 
>> waffle_display_info_json
>> +struct waffle_display 
>> *self
>> +bool platform_info
>> +  
>> +
>> +  
>>  bool 
>> waffle_display_supports_context_api
>>  struct waffle_display 
>> *self
>>  int32_t context_api
>> @@ -129,6 +136,18 @@ struct waffle_display;
>>
>>
>>
>> +waffle_display_info_json()
>> +
>> +  
>> +Return a JSON string containing information about the current 
>> context on the given display, including Waffle platform and API, GL 
>> version/vendor/renderer and extensions.
>> +If platform_info is true, 
>> platform-specific information (such as GLX or EGL versions and extensions) 
>> will be included as available.
>> +Returns NULL on error.
>> +The string should be deallocated with 
>> free3.
>> +  
>> +
>> +  
>> +
>> +  
>>  
>> waffle_display_supports_context_api()
>>  
>>
>> diff --git a/src/waffle/api/waffle_display.c 
>> b/src/waffle/api/waffle_display.c
>> index fa19462..7abe2ef 100644
>> --- a/src/waffle/api/waffle_display.c
>> +++ b/src/waffle/api/waffle_display.c
>> @@ -23,13 +23,61 @@
>>  // OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF 
>> THE USE
>>  // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>
>> +#include 
>> +#include 
>> +
>>  #include "api_priv.h"
>>
>> -#include "wcore_error.h"
>> +#include "json.h"
>> +
>> +#include "wcore_context.h"
>>  #include "wcore_display.h"
>> +#include "wcore_error.h"
>>  #include "wcore_platform.h"
>>  #include "wcore_util.h"
>>
>> +typedef unsigned int GLint;
>> +typedef unsigned int GLenum;
>> +typedef unsigned char GLubyte;
>> +
>> +enum {
>> +// Copied from .
>> +GL_NO_ERROR = 0,
>> +
>> +GL_CONTEXT_FLAGS = 0x821e,
>> +GL_CONTEXT_FLAG_FORWARD_COMPATIBLE_BIT = 0x0001,
>> +GL_CONTEXT_FLAG_DEBUG_BIT  = 0x0002,
>> +GL_CONTEXT_FLAG_ROBUST_ACCESS_BIT_ARB  = 0x0004,
>> +
>> +GL_VENDOR  = 0x1F00,
>> +GL_RENDERER= 0x1F01,
>> +GL_VERSION = 0x1F02,
>> +GL_EXTENSIONS  = 0x1F03,
>> +GL_NUM_EXTENSIONS  = 0x821D,
>> +GL_SHADING_LANGUAGE_VERSION= 0x8B8C,
>> +};
>> +
>> +#ifndef _WIN32
>> +#define APIENTRY
>> +#else
>> +#ifndef APIENTRY
>> +#define APIENTRY __stdcall
>> +#endif
>> +#endif
>> +
>> +static GLenum (APIENTRY *glGetError)(void);
>> +static void (APIENTRY *glGetIntegerv)(GLenum pname, GLint *params);
>> +static const GLubyte * (APIENTRY *glGetString)(GLenum name);
>> +static const GLubyte * (APIENTRY *glGetStringi)(GLenum name, GLint i);
>> +
>> +#if defined(__GNUC__)
>> +#define NORETURN __attribute__((noreturn))
>> +#elif defined(_MSC_VER)
>> +#define 

Re: [waffle] [PATCH 04/12] core: add JSON library

[Frank Henigman]

On Fri, Apr 8, 2016 at 7:38 PM, Chad Versace  wrote:
> On 01/08/2016 04:17 AM, Emil Velikov wrote:
>> On 6 January 2016 at 21:56, Frank Henigman  wrote:
>>> A small library for building JSON strings.
>>>
>>> Signed-off-by: Frank Henigman 
>>> ---
>>>  src/waffle/CMakeLists.txt |   1 +
>>>  src/waffle/core/json.c| 235 
>>> ++
>>>  src/waffle/core/json.h|  93 ++
>
>> Is the library is copied/derived from another project or written from
>> scratch ? If the former should we move it to third_party/  ?
>
> I have the same question. Was this code copied from a public Google project?
> If so, the we should place in third_party/ with a note explaining its origin
> and explaining how to update it in the future. If not, the the current 
> location
> in src/waffle/core/ is appropriate.

I wrote it.  I didn't really want to write a json lib, but I did want
the json-building code (the code that calls a json lib) to be as clear
and simple as I could make it, and it seemed I needed to write
something to facilitate that.  I'm open to a third party solution, if
it doesn't require uglier code to use it.  I didn't think of it at the
time, but maybe a little wrapping around some third party lib could do
the job.  But by now I don't remember anything about the third party
libs I looked at.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle