H. Verbeet wrote:
2008/7/24 Vitaliy Margolen wine-devel@kievinfo.com:
side affects. But the full version of Psychonauts crashes after initial videos. Same as it did when we had problems with multi-threaded games.
Is that a regression introduced by the patch?
Yes. Without the patch everything works. With the patch it crashes.
It's a bit unfortunate that it only happens in the full version. Have you got any idea which part of that patch is causing the crash? Is it the Clear call again? In the original case it was pretty clear that calling apply_fbo_state() without activating the context for the current thread caused issues, but as far as I can see that part should still work correctly with my patch.
It's this part:
diff --git a/dlls/wined3d/drawprim.c b/dlls/wined3d/drawprim.c index 9a47f8b..f2ab699 100644 --- a/dlls/wined3d/drawprim.c +++ b/dlls/wined3d/drawprim.c @@ -974,6 +974,7 @@ void drawPrimitive(IWineD3DDevice *iface,
/* Signals other modules that a drawing is in progress and the stateblock finalized */ This->isInDraw = TRUE; + ActivateContext(This, This->render_targets[0], CTXUSAGE_DRAWPRIM);
/* Ok, we will be updating the screen from here onwards so grab the lock */
@@ -983,7 +984,6 @@ void drawPrimitive(IWineD3DDevice *iface, LEAVE_GL(); }
- ActivateContext(This, This->render_targets[0], CTXUSAGE_DRAWPRIM); ENTER_GL();
if (This->stencilBufferTarget) {
Vitaliy Margolen wrote:
H. Verbeet wrote:
2008/7/24 Vitaliy Margolen wine-devel@kievinfo.com:
side affects. But the full version of Psychonauts crashes after initial videos. Same as it did when we had problems with multi-threaded games.
Is that a regression introduced by the patch?
Yes. Without the patch everything works. With the patch it crashes.
It's a bit unfortunate that it only happens in the full version. Have you got any idea which part of that patch is causing the crash? Is it the Clear call again? In the original case it was pretty clear that calling apply_fbo_state() without activating the context for the current thread caused issues, but as far as I can see that part should still work correctly with my patch.
It's this part:
diff --git a/dlls/wined3d/drawprim.c b/dlls/wined3d/drawprim.c index 9a47f8b..f2ab699 100644 --- a/dlls/wined3d/drawprim.c +++ b/dlls/wined3d/drawprim.c @@ -974,6 +974,7 @@ void drawPrimitive(IWineD3DDevice *iface,
/* Signals other modules that a drawing is in progress and thestateblock finalized */ This->isInDraw = TRUE;
ActivateContext(This, This->render_targets[0], CTXUSAGE_DRAWPRIM);
/* Ok, we will be updating the screen from here onwards so grab the
lock */
@@ -983,7 +984,6 @@ void drawPrimitive(IWineD3DDevice *iface, LEAVE_GL(); }
ActivateContext(This, This->render_targets[0], CTXUSAGE_DRAWPRIM); ENTER_GL();
if (This->stencilBufferTarget) {
BTW this is a driver crash in libglcore.so with what appears to be a null-pointer dereference. I'm trying to play with your code to see how to "fix" it.
Vitaliy.
2008/7/25 Vitaliy Margolen wine-devel@kievinfo.com:
BTW this is a driver crash in libglcore.so with what appears to be a null-pointer dereference. I'm trying to play with your code to see how to "fix" it.
Thanks.
H. Verbeet wrote:
2008/7/25 Vitaliy Margolen wine-devel@kievinfo.com:
BTW this is a driver crash in libglcore.so with what appears to be a null-pointer dereference. I'm trying to play with your code to see how to "fix" it.
Thanks.
Ok I think I see the problem: 0045:trace:d3d_surface:surface_load_ds_location before bind_fbo 0045:trace:d3d:bind_fbo 0x14f474 -> 0 0045:trace:d3d:bind_fbo glGenFramebuffersEXT() -> 2 @ ../../../wine.git/dlls/wined3d/device.c:6084 call ok 0045:trace:d3d:bind_fbo glBindFramebuffer(0x8d40 2) @ ../../../wine.git/dlls/wined3d/device.c:6087 call ok 0045:trace:d3d_surface:surface_load_ds_location after bind_fbo
... 0033:trace:d3d_surface:surface_load_ds_location before bind_fbo 0033:trace:d3d:bind_fbo 0x14f474 -> 2 0033:trace:seh:raise_exception code=c0000005 flags=0 addr=0x7d05bc3f 0033:trace:seh:raise_exception info[0]=00000000 0033:trace:seh:raise_exception info[1]=00000028
So it seems the FBOs are created in one thread used there, then rebounding in the new thread leads to the crash.
Any ideas how to fix this? Or what to try?
Vitaliy.
2008/7/25 Vitaliy Margolen wine-devel@kievinfo.com:
Ok I think I see the problem: 0045:trace:d3d_surface:surface_load_ds_location before bind_fbo 0045:trace:d3d:bind_fbo 0x14f474 -> 0 0045:trace:d3d:bind_fbo glGenFramebuffersEXT() -> 2 @ ../../../wine.git/dlls/wined3d/device.c:6084 call ok 0045:trace:d3d:bind_fbo glBindFramebuffer(0x8d40 2) @ ../../../wine.git/dlls/wined3d/device.c:6087 call ok 0045:trace:d3d_surface:surface_load_ds_location after bind_fbo
... 0033:trace:d3d_surface:surface_load_ds_location before bind_fbo 0033:trace:d3d:bind_fbo 0x14f474 -> 2 0033:trace:seh:raise_exception code=c0000005 flags=0 addr=0x7d05bc3f 0033:trace:seh:raise_exception info[0]=00000000 0033:trace:seh:raise_exception info[1]=00000028
So it seems the FBOs are created in one thread used there, then rebounding in the new thread leads to the crash.
Any ideas how to fix this? Or what to try?
AFAIK that's supposed to be legal and work, but it might be worth it to try to trigger the issue in a standalone GL progam.
On Sat, Jul 26, 2008 at 1:48 AM, H. Verbeet hverbeet@gmail.com wrote:
2008/7/25 Vitaliy Margolen wine-devel@kievinfo.com:
So it seems the FBOs are created in one thread used there, then rebounding in the new thread leads to the crash.
Any ideas how to fix this? Or what to try?
AFAIK that's supposed to be legal and work, but it might be worth it to try to trigger the issue in a standalone GL progam.
Is this an Nvidia card? Last I checked, starting with the 100.xx drivers (on Linux anyway), trying to bind the same FBO on different contexts in different threads would cause a crash. The "recommended" workaround seems to be to create separate FBOs for each context/thread.
Allan Tong wrote:
On Sat, Jul 26, 2008 at 1:48 AM, H. Verbeet hverbeet@gmail.com wrote:
2008/7/25 Vitaliy Margolen wine-devel@kievinfo.com:
So it seems the FBOs are created in one thread used there, then rebounding in the new thread leads to the crash.
Any ideas how to fix this? Or what to try?
AFAIK that's supposed to be legal and work, but it might be worth it to try to trigger the issue in a standalone GL progam.
Is this an Nvidia card? Last I checked, starting with the 100.xx drivers (on Linux anyway), trying to bind the same FBO on different contexts in different threads would cause a crash. The "recommended" workaround seems to be to create separate FBOs for each context/thread.
Yes this is Nvidia card. And I think I've seen something about this on the Internet.
Also this seems to be related to your bug http://bugs.winehq.org/show_bug.cgi?id=9973 as well. Where it causes some rendering problems but not a crash.
So if we need to create an fbo for each thread, does that mean that everything needs to be rebound to it on the context switch?
Vitaliy
2008/7/26 Vitaliy Margolen wine-devel@kievinfo.com:
So if we need to create an fbo for each thread, does that mean that everything needs to be rebound to it on the context switch?
It would mostly mean that apply_fbo_state() would need to track things per-context rather than per-device. I guess that would make things a bit more like eg. pbuffers. We'd also need a per-context src_fbo and dst_fbo for things like stretch_rect_fbo(). Unless I misunderstood, this sounds more like a driver bug than an issue with the way we use FBOs though.
On Sat, Jul 26, 2008 at 3:39 PM, H. Verbeet hverbeet@gmail.com wrote:
2008/7/26 Vitaliy Margolen wine-devel@kievinfo.com:
So if we need to create an fbo for each thread, does that mean that everything needs to be rebound to it on the context switch?
It would mostly mean that apply_fbo_state() would need to track things per-context rather than per-device. I guess that would make things a bit more like eg. pbuffers. We'd also need a per-context src_fbo and dst_fbo for things like stretch_rect_fbo(). Unless I misunderstood, this sounds more like a driver bug than an issue with the way we use FBOs though.
You could try the attached hack patch to see if it fixes the problem you're seeing. It completely ignores FBO cleanup, but it should avoid the driver bug.
2008/7/26 Allan Tong actong88@gmail.com:
You could try the attached hack patch to see if it fixes the problem you're seeing. It completely ignores FBO cleanup, but it should avoid the driver bug.
It's not *that* easy. You'll need to adjust the code in apply_fbo_state() as well. fbo_color_attachments and fbo_depth_attachment would need to be moved to the context.
On Sat, Jul 26, 2008 at 5:34 PM, H. Verbeet hverbeet@gmail.com wrote:
2008/7/26 Allan Tong actong88@gmail.com:
You could try the attached hack patch to see if it fixes the problem you're seeing. It completely ignores FBO cleanup, but it should avoid the driver bug.
It's not *that* easy. You'll need to adjust the code in apply_fbo_state() as well. fbo_color_attachments and fbo_depth_attachment would need to be moved to the context.
Yes, you're quite right.
-
Allan Tong -
H. Verbeet -
Vitaliy Margolen