On Wednesday, February 22, 2017 10:35:24 AM PST Robert Bragg wrote:
> On Wed, Feb 22, 2017 at 1:24 AM, Kenneth Graunke <kenn...@whitecape.org>
> wrote:
> > On Wednesday, February 15, 2017 1:37:36 PM PST Robert Bragg wrote:
[snip]
> >> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> >> index f3a24df589..e6cf1f4af6 100644
> >> --- a/src/mesa/main/mtypes.h
> >> +++ b/src/mesa/main/mtypes.h
> >> @@ -1860,6 +1860,23 @@ struct gl_perf_monitor_group
> >>
> >>
> >> /**
> >> + * A query object instance as described in INTEL_performance_query.
> >> + *
> >> + * NB: We want to keep this and the corresponding backend structure
> >> + * relatively lean considering that applications may expect to
> >> + * allocate enough objects to be able to query around all draw calls
> >> + * in a frame.
> >> + */
> >> +struct gl_perf_query_object
> >> +{
> >> + GLuint Id; /**< hash table ID/name */
> >> + GLuint Used:1; /**< has been used for 1 or more queries */
> >> + GLuint Active:1; /**< inside Begin/EndPerfQuery */
> >> + GLuint Ready:1; /**< result is ready? */
> >
> > Please use "unsigned Id" and "bool Used:1" - we're trying to get away
> > from GL type aliases when not directly API-facing.
>
> Ah right I was generally aware of that but doing a skimming everything
> with this in mind I found a few other little bits to clean up though I
> ended having some second thoughts about these particular members:
>
> This Id is a record of the GLuint ID given to the application, just
> used for debugging currently. The value is returned by
> _mesa_HashFindFreeKeyBlock() which is currently implemented in terms
> of the GLuint type. One other place where we access the same ID for
> debugging is via _mesa_HashWalk() which takes a callback expecting a
> GLuint argument. I can still change, but when I thought about this it
> felt like it was indeed a directly api facing value.
>
> For the bitfields I started over thinking what it means to have a bool
> bitfield since I doubted whether it could be assumed to be unsigned
> and then wondered about the potential for a bug with some code trying
> to compare a bitfield value == 1, or indexing an array. Does Mesa
> require a c99 compiler, otherwise I don't think it's unheard of for
> bool to end up as a signed int typedef. Anyway, besides the
> overly-pedantic thought, I guessed you wouldn't really mind me using
> "unsigned Used:1;" for the sake of avoiding GLuint. I don't think it
> would make a practical difference since the struct will be naturally
> padded to 8 bytes in all likelyhood either way. I'll prod you on IRC
> to check if you really have a strong opinion here before I push.We assume bool types become 0 or 1 when cast to integers, as guaranteed by C99. There are existing examples of bool:1 in NIR, i965, and vc4. I think it should be OK in Mesa core. But, feel free to use unsigned. Or GLuint when you have a rationale for doing so, as above. A lot of people just use GL types everywhere for no particular reason, but it makes some sense here. --Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
