Re: [waffle] json, approach 2, version 2

2016-06-18 Thread Frank Henigman
Perhaps I rewrote the branch after posting that link, it appears not
to be on the json5 branch anymore.  Though the link should still take
you to original (now orphaned) commit.
In any case, you and Emil answered my question.  I'll assume I did the
glxinfo correctly for now, and you'll see it when I send v2 of the
series.  I'll send v2 after rebasing and incorporating the latest
suggestions.
Thanks.

On Tue, Jun 14, 2016 at 12:13 PM, Chad Versace  wrote:
> On Thu 21 Apr 2016, Frank Henigman wrote:
>> Thanks Emil and Chad for reviewing my json series.  All suggestions
>> implemented in v2, except where I replied inline.  I'll hold off
>> sending in case there's more back-and-forth over the first set of
>> comments.  Would also be nice if Chad merged his get-current branch
>> into master, as I use it in v2.
>
>> Not sure if I did the right thing with glx info.  Seems like all three
>> sections (server, client, common) show about the same list of
>> extensions.  That can wait until I send v2, or if anyone wants to look
>> now:
>> https://github.com/fjhenigman/waffle/commit/b358ac50c00ce51fae6546b1e96c9adc32fcbdc7
>
> Hi Frank,
>
> I've returned from sabbatical, and am now trying to catch up with
> everything.
>
> I can't find the above sha1 in your repo. Should I be examining your
> json5 branch?
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] json, approach 2, version 2

2016-06-14 Thread Chad Versace
On Thu 21 Apr 2016, Frank Henigman wrote:
> Thanks Emil and Chad for reviewing my json series.  All suggestions
> implemented in v2, except where I replied inline.  I'll hold off
> sending in case there's more back-and-forth over the first set of
> comments.  Would also be nice if Chad merged his get-current branch
> into master, as I use it in v2.

Hi Frank, I think now is a good time to send v2.

Since the patches are not on the list yet, I'll say a few comments here:

core: add JSON library
Overall, looks good. The only problem I see is
a potential buffer overflow in put().

waffle: add waffle_display_info_json()
For simplicity's sake, please drop the 'platform_too'
parameter to waffle_display_info_json(). Waffle should
return all the info, and the client the should ignore
any keys (such as platform-specific keys) that it does
not recognize.

The patch has some problems regarding "current-ness".
It says the new function "returns a JSON string
containing information about the current context on the
given display". As defined by EGL and GLX, though,
current-ness is a mapping of (thread) -> (display,
context, surfaces), and not a mapping of
(thread, display) -> (context, surfaces).

As a consequence, the waffle_display_info_json()
signature is incorrect because due to its 'display'
parameter. If the function has a display parameter, then
that allows the user to do undefined things such as
below:

struct waffle_display *dpy1 = ...;
struct waffle_display *ctx1 = ...;

struct waffle_display *dpy2 = ...;
struct waffle_display *ctx2 = ...;

waffle_make_current(dpy1, NULL, ctx1);
// use ctx1
waffle_make_current(dpy2, NULL, ctx2);
// use ctx2

// Now dpy2 and ctx2 are bound to the thread.
// So what should waffle_display_info_json() do
// when given dpy1?
waffle_display_info_json(dpy1);

wflinfo: another option for JSON output
As far as I can tell, the two JSON formats are identical
except for formatting, as they should be :). Since
they're identical, I don't understand the need to add
the new format 'json2'.

At the end of this patch series, your json is Waffle's
*real* json. So I expected this patch to remove Dylan's
json code from wflinfo.c and replace it with yours. Then
--format=json would return your json.

> When comparing my json output to the landed json output I noticed that
> the landed version omits the context flags found in the old format.
> Was that deliberate?  If so I'll remove it from my json.

It's an accident that wflinfo does not currently report the context
flags. I'd like to see them in the wflinfo output.

> Not sure if I did the right thing with glx info.  Seems like all three
> sections (server, client, common) show about the same list of
> extensions.  That can wait until I send v2, or if anyone wants to look
> now:
> https://github.com/fjhenigman/waffle/commit/b358ac50c00ce51fae6546b1e96c9adc32fcbdc7

As Emil said earlier: wflinfo should report all three sections, as they
do sometimes differ.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] json, approach 2, version 2

2016-06-14 Thread Chad Versace
On Thu 21 Apr 2016, Frank Henigman wrote:
> Thanks Emil and Chad for reviewing my json series.  All suggestions
> implemented in v2, except where I replied inline.  I'll hold off
> sending in case there's more back-and-forth over the first set of
> comments.  Would also be nice if Chad merged his get-current branch
> into master, as I use it in v2.

> Not sure if I did the right thing with glx info.  Seems like all three
> sections (server, client, common) show about the same list of
> extensions.  That can wait until I send v2, or if anyone wants to look
> now:
> https://github.com/fjhenigman/waffle/commit/b358ac50c00ce51fae6546b1e96c9adc32fcbdc7

Hi Frank,

I've returned from sabbatical, and am now trying to catch up with
everything.

I can't find the above sha1 in your repo. Should I be examining your
json5 branch?
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] json, approach 2, version 2

2016-05-02 Thread Chad Versace
On Thu 21 Apr 2016, Frank Henigman wrote:

[snip]

> When comparing my json output to the landed json output I noticed that
> the landed version omits the context flags found in the old format.
> Was that deliberate?  If so I'll remove it from my json.

The JSON should list the context flags. It's an accident that wflinfo
currently doesn't.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle


Re: [waffle] json, approach 2, version 2

2016-04-26 Thread Emil Velikov
On 21 April 2016 at 21:27, Frank Henigman  wrote:
> Thanks Emil and Chad for reviewing my json series.  All suggestions
> implemented in v2, except where I replied inline.  I'll hold off
> sending in case there's more back-and-forth over the first set of
> comments.

> Would also be nice if Chad merged his get-current branch
> into master, as I use it in v2.
>
Would be nice to see the series fly by the ML, although from a quick
look they look great.
We could use it to resolve some nasty implementations details in
nacl/android/others and get GBM's window_resize working.

> When comparing my json output to the landed json output I noticed that
> the landed version omits the context flags found in the old format.
> Was that deliberate?  If so I'll remove it from my json.
>
I'll refer Chad to that one.

> Not sure if I did the right thing with glx info.  Seems like all three
> sections (server, client, common) show about the same list of
> extensions.
That may be true for some vendors, but it's not the rule afaict. On my
systems running mesa + i965 and nouveau the lists do differ.
How useful the separate lists are is another topic ;-)

> That can wait until I send v2, or if anyone wants to look
> now:
> https://github.com/fjhenigman/waffle/commit/b358ac50c00ce51fae6546b1e96c9adc32fcbdc7
I've not looked in details in the github branch, but considering that
wcore_[cm]alloc is used (instead of [cm]alloc or strdup), the leak is
plugged and people are happy with the other two topics (loose common
GL data, even if we fail in print_platform_specific) (don't print
anything and/or error if print_platform_specific() is not
implemented), I'd say just do with the series.

Reviewed-by: Emil Velikov 

Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle