On Thu, Aug 9, 2012 at 7:47 AM, Brian Paul <bri...@vmware.com> wrote:
> On 07/23/2012 10:59 AM, Jordan Justen wrote:
>>
>> Signed-off-by: Jordan Justen<jordan.l.jus...@intel.com>
>> ---
>> src/mesa/main/pack.c | 549
>> +++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 547 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/pack.c b/src/mesa/main/pack.c
>> index efbff5d..a599f21 100644
>> --- a/src/mesa/main/pack.c
>> +++ b/src/mesa/main/pack.c
>> @@ -521,6 +521,8 @@ _mesa_pack_rgba_span_from_uints(struct gl_context
>> *ctx, GLuint n, GLuint rgba[][
>> GLenum dstFormat, GLenum dstType,
>> GLvoid *dstAddr)
>> {
>> + GLuint i;
>> +
>> switch(dstType) {
>> case GL_UNSIGNED_INT:
>> pack_uint_from_uint_rgba(ctx, dstAddr, dstFormat, rgba, n);
>> @@ -540,7 +542,278 @@ _mesa_pack_rgba_span_from_uints(struct gl_context
>> *ctx, GLuint n, GLuint rgba[][
>> case GL_BYTE:
>> pack_byte_from_uint_rgba(ctx, dstAddr, dstFormat, rgba, n);
>> break;
>> -
>> + case GL_UNSIGNED_BYTE_3_3_2:
>> + if ((dstFormat == GL_RGB) || (dstFormat == GL_RGB_INTEGER)) {
>> + GLubyte *dst = (GLubyte *) dstAddr;
>> + for (i=0;i<n;i++) {
>> + dst[i] = (CLAMP(rgba[i][RCOMP], 0, 7)<< 5)
>> + | (CLAMP(rgba[i][GCOMP], 0, 7)<< 2)
>> + | (CLAMP(rgba[i][BCOMP], 0, 3) );
>> + }
>> + }
>
> I think we should have an else clause here and in similar places below to
> catch unexpected format/type combinations:
>
> else {
> _mesa_problem(ctx, "unexpected dstFormat/dstType in
> _mesa_pack_rgba_span_from_uints()");
> }
>
> Actually, since there'd be many instances of this, maybe use a 'goto
> badformattype' error handler at the end of the function.
That sounds good. I'll make these changes.
> Otherwise, the patch looks good (though I didn't inspect every line of
> clamping/shifting in detail. Do we have a piglit tests that hit all (most?)
> of these cases?
These were developed in the context of adding rgb10_a2ui texture
support. I did not think that piglit adequately tested this texture
format, nor integer textures in general. (Or, perhaps I just missed
the tests.) I relied upon Intel's 'oglconform' test tool to increase
the test scope.
So, no, unfortunately I don't believe piglit will hit many of these cases.
-Jordan
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
