Bug 34047 - Assert in _tnl_import_array() when using GLfixed vertex datatypes with GLESv2
Summary: Assert in _tnl_import_array() when using GLfixed vertex datatypes with GLESv2
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Other (show other bugs)
Version: 7.9
Hardware: x86 (IA32) other
: medium critical
Assignee: Chad Versace
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-08 10:27 UTC by David Leong
Modified: 2011-02-15 15:42 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:

Attachments
Proposed fix (2.31 KB, patch)
2011-02-14 15:15 UTC, Chad Versace 		Details | Splinter Review
View All

Description David Leong 2011-02-08 10:27:53 UTC 
-Hardware/software setup:
Intel GMA 3150 (Atom N450) DRI i915
Meego 1.1 trunk 
Mesa 7.9.1 / Mesa 7.10 

Our program hits an assert in _tnl_import_array() when we try to use GLfixed datatypes for vertices. It seems the _tnl_import_array() method is unable to convert this datatype and just asserts to crash the program. According to the GLESv2 spec, GLfixed datatypes are supported for vertex input. 

We are using the following gl functions to render:
glVertexAttribPointer();  // Datatype is GL_FIXED
glDrawElements(); 

I prototyped a GLfixed to GLfloat conversion function in _tnl_import_array()and it seems to have fixed the problem. 

static void
convert_fixed_to_float(const struct gl_client_array *input,
                       const GLubyte *ptr, GLfloat *fptr,
                       GLuint count, GLuint sz)
{
    GLuint i,j;
    GLfixed *in =(GLfixed *)ptr;
    for (i = 0; i < count; i++) {
        for (j = 0; j < input->Size; j++) {
            *fptr = (GLfloat)((*in)*(1/65536.0f));
            in++;
            fptr++;
        }
        ptr += input->StrideB;
    }
}
Comment 1 David Leong 2011-02-08 10:30:24 UTC 
Just to add a note. The same code using GL_FIXED works correctly when using the SGX driver.
Comment 2 David Leong 2011-02-08 10:59:04 UTC 
Code also works ok on the Symbian GLESv2 and windows GLESv2 emulation implementations.
Comment 3 Chad Versace 2011-02-08 14:35:35 UTC 
Assign bug to myself: chad@chad-versace.us
Comment 4 Chad Versace 2011-02-08 16:22:06 UTC 
(In reply to comment #0)
> I prototyped a GLfixed to GLfloat conversion function in _tnl_import_array()and
> it seems to have fixed the problem. 

I have prototyped a similiar fix, but I cannot test it yet. On my machine the fix just causes the assert to occur deeper in the graphics stack, in the DRI driver. Once I fix the i965 DRI driver, I'll report back.
Comment 5 Chad Versace 2011-02-14 15:15:42 UTC 
Created attachment 43357 [details] [review]
Proposed fix

David, please try the attached patch. It fixes my test case for GL_FIXED.

You mentioned that with your fix, visual artifacts were present. Are they still present with this patch?
Comment 6 David Leong 2011-02-14 15:44:00 UTC 
Hi Chad,

Thank you for fixing the bug. Your patch does indeed fix all the problems. Excellent!
Comment 7 Chad Versace 2011-02-14 15:56:51 UTC 
(In reply to comment #6)
> Hi Chad,
> 
> Thank you for fixing the bug. Your patch does indeed fix all the problems.
> Excellent!

Great. I just submitted the patch for review. It should hit master Wednesday morning if no forsees a problem.
Comment 8 David Leong 2011-02-14 16:04:39 UTC 
Backport please to mesa 7.9.1 on Meego?
Comment 9 Chad Versace 2011-02-15 10:10:06 UTC 
(In reply to comment #8)
> Backport please to mesa 7.9.1 on Meego?

Oops, forgot about that. I'll mark it as a backport candidate.

I can't say exactly when it will be backported. Ian Romanick decides that.
Comment 10 Chad Versace 2011-02-15 15:42:35 UTC 
Marking RESOLVED/FIXED.
Fixed by:

commit a231ac23f41a38cf9bde80bab4cb6aa8821d4895
Author: Chad Versace <chad.versace@intel.com>
Date:   Tue Feb 15 15:30:05 2011 -0800

    tnl: Add support for datatype GL_FIXED in vertex arrays

----

David, it is marked as a backport candidate for 7.9. Ian said backporting should occur in about two weeks.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.