Bug 96950 - Another regression from bc4e0c486: vbo: Use a bitmask to track the active arrays in vbo_exec*.
Summary: Another regression from bc4e0c486: vbo: Use a bitmask to track the active arr...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-15 23:13 UTC by Brian Paul
Modified: 2016-07-27 14:14 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:

Attachments
Patch for debugging (1.69 KB, patch)
2016-07-16 08:44 UTC, Mathias Fröhlich 		Details | Splinter Review
Fix for an other assert in immediate mode rendering with the linux turbine (2.71 KB, patch)
2016-07-22 13:35 UTC, Mathias Fröhlich 		Details | Splinter Review
Show Obsolete (1) View All

Description Brian Paul 2016-07-15 23:13:33 UTC 
I'm seeing a failed assertion at vbo_exec_api.c:183 that started with commit bc4e0c486.  It occurs with Redway3D's Turbine demo (I think it's Windows only).

I've taken a quick look but haven't found the cause.  I may take a look in a week when I'm back from vacation.
Comment 1 Rob Clark 2016-07-15 23:19:28 UTC 
Do you still see the issue after 64d35f81 "vbo: fix attr reset"?
Comment 2 Brian Paul 2016-07-15 23:20:17 UTC 
(In reply to Rob Clark from comment #1)
> Do you still see the issue after 64d35f81 "vbo: fix attr reset"?

Yes.
Comment 3 Mathias Fröhlich 2016-07-16 08:44:27 UTC 
Created attachment 125104 [details] [review]
Patch for debugging
Comment 4 Mathias Fröhlich 2016-07-16 08:44:43 UTC 
The remaining thing that I could see to get into this situation would be that  newSize is equal to zero on entry to in vbo_exec_wrap_upgrade_vertex.
The only case where I could not rule out newSize being zero just now is with evaluators.
If this is our problem, the attached patch may bring us to a backtrace that matters.
Can you help me verify that the above thought heads into the right direction?

Mathias
Comment 5 Mathias Fröhlich 2016-07-19 05:14:31 UTC 
Or Can you provide an apitrace?
I have no such system to reproduce at hands.
Comment 6 Mathias Fröhlich 2016-07-22 13:35:15 UTC 
Created attachment 125255 [details] [review]
Fix for an other assert in immediate mode rendering with the linux turbine

Brian,

I tried to reproduce the described problem with the linux demo and stepped ontpo an other assert in vbo_exec_vtx_wrap(). This assert may be related to the one you mentioned. With this patch applied the linux turbine demo runs fine on i965.
Can you check if the windows demo gets fixed by this patch too?

Rob,

The patch fixes the problem you observed with 0ad in a different way. Can you check if this approach also fixes your problem?

Thanks

Mathias
Comment 7 Rob Clark 2016-07-25 20:49:21 UTC 
(In reply to Mathias Fröhlich from comment #6)
> Created attachment 125255 [details] [review] [review]
> Fix for an other assert in immediate mode rendering with the linux turbine
> 
> Rob,
> 
> The patch fixes the problem you observed with 0ad in a different way. Can
> you check if this approach also fixes your problem?

it does not seem to regress 0ad..

fwiw, if you are curious, this should reproduce my original problem (if you revert my earlier fix and don't apply your patch)

https://people.freedesktop.org/~robclark/0ad-cycladic-archipelago.trace.xz
Comment 8 Brian Paul 2016-07-25 21:22:14 UTC 
(In reply to Mathias Fröhlich from comment #6)
> Created attachment 125255 [details] [review] [review]
> Fix for an other assert in immediate mode rendering with the linux turbine
> 
> Brian,
> 
> I tried to reproduce the described problem with the linux demo and stepped
> ontpo an other assert in vbo_exec_vtx_wrap(). This assert may be related to
> the one you mentioned. With this patch applied the linux turbine demo runs
> fine on i965.
> Can you check if the windows demo gets fixed by this patch too?

Yeah, your patch seems to fix things.  Thanks!

Tested-by: Brian Paul <brianp@vmware.com>

As for the patch itself, I think we want to s/boolean/bool/ and maybe #include <stdbool.h> just to be safe.

Otherwise, Reviewed-by: Brian Paul <brianp@vmware.com>
Comment 9 Mathias Fröhlich 2016-07-27 04:48:30 UTC 
I have verified 0ad with the provided trace here, updated as requested and pushed.
Thanks for testing and review on your side.

I assume the originator verifies and closes the bug?

Mathias
Comment 10 Brian Paul 2016-07-27 14:14:02 UTC 
Verified.  Thanks!

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.