Am Sonntag, 25. November 2007 00:07:15 schrieb Allan Tong:
Fixes failures in visual tests for ddraw and d3d8 when using FBO offscreen rendering mode.
- Allan
This doesn't look right to me. It will most likely break overall FBO support.
There is indeed a problem with the fbo and ActivateContext integration, but fixing this is more involved than just setting the framebuffer to 0.
On Nov 25, 2007 7:15 AM, Stefan Dösinger stefan@codeweavers.com wrote:
Am Sonntag, 25. November 2007 00:07:15 schrieb Allan Tong:
Fixes failures in visual tests for ddraw and d3d8 when using FBO offscreen rendering mode.
- Allan
This doesn't look right to me. It will most likely break overall FBO support.
There is indeed a problem with the fbo and ActivateContext integration, but fixing this is more involved than just setting the framebuffer to 0.
Any hints where to start looking? The way I understand it, ActivateContext will only try to set the draw buffer to GL_FRONT, GL_BACK, or GL_AUX0, which should mean that at some point before that, the framebuffer would need to be set to 0, or do I have that wrong? Or is it that I didn't reset the framebuffer back to its previous value?
- Allan
Am Sonntag, 25. November 2007 18:13:43 schrieb Allan Tong:
On Nov 25, 2007 7:15 AM, Stefan Dösinger stefan@codeweavers.com wrote:
Am Sonntag, 25. November 2007 00:07:15 schrieb Allan Tong:
Fixes failures in visual tests for ddraw and d3d8 when using FBO offscreen rendering mode.
- Allan
This doesn't look right to me. It will most likely break overall FBO support.
There is indeed a problem with the fbo and ActivateContext integration, but fixing this is more involved than just setting the framebuffer to 0.
Any hints where to start looking? The way I understand it, ActivateContext will only try to set the draw buffer to GL_FRONT, GL_BACK, or GL_AUX0, which should mean that at some point before that, the framebuffer would need to be set to 0, or do I have that wrong? Or is it that I didn't reset the framebuffer back to its previous value?
The framebuffer setup should be merged with ActivateContext, but not by simply setting the framebuffer to 0. Currently there are two issues: the one you are seeing(the complaints from opengl), and multithreading issues.
Parts of the code call ActivateContext before setting the fbo: In this case you can get errors in glDrawBuffers. Other parts call the fbo setup before calling ActivateContext. In this case the app can crash after a rendering thread switch because no GL context is active.
The whole issue is pretty complex unfortunately. One issue is that we cannot select context and drawable separately for onscreen rendering, and pbuffer offscreen rendering. In the case of fbos, the drawable can be set without changing the context.
Another issue is that ActivateContext deals with one buffer only, but with fbo we can have multiple simultaneous ones(GL_ARB_draw_buffers, or multiple render targets in d3d speak). In case of onscreen rendering only one render target may be active from the d3d side, and in case of back/aux and pbuffer offscreen rendering we're limited to one target from the opengl side.
What has to be done in some way is to merge apply_fbo_state() with ActivateContext. My original idea was to select the primary target and the depthstencil buffer in ActivateContext similarly to the existing code, and have higher render targets set in the state manager. Unfortunately drivers do not like that, and we have to set all color and depth attachments of the fbo at once.
On Nov 25, 2007 7:15 AM, Stefan Dösinger stefan@codeweavers.com
wrote:
Am Sonntag, 25. November 2007 00:07:15 schrieb Allan Tong:
Fixes failures in visual tests for ddraw and d3d8 when using FBO offscreen rendering mode.
- Allan
This doesn't look right to me. It will most likely break overall FBO support.
There is indeed a problem with the fbo and ActivateContext
integration,
but fixing this is more involved than just setting the framebuffer to
Any hints where to start looking? The way I understand it, ActivateContext will only try to set the draw buffer to GL_FRONT, GL_BACK, or GL_AUX0, which should mean that at some point before that, the framebuffer would need to be set to 0, or do I have that wrong? Or is it that I didn't reset the framebuffer back to its previous value?
The framebuffer setup should be merged with ActivateContext, but not by simply setting the framebuffer to 0. Currently there are two issues: the one you are seeing(the complaints from opengl), and multithreading issues.
Parts of the code call ActivateContext before setting the fbo: In this case you can get errors in glDrawBuffers. Other parts call the fbo setup before calling ActivateContext. In this case the app can crash after a rendering thread switch because no GL context is active.
The whole issue is pretty complex unfortunately. One issue is that we cannot select context and drawable separately for onscreen rendering, and pbuffer offscreen rendering. In the case of fbos, the drawable can be set without changing the context.
Another issue is that ActivateContext deals with one buffer only, but with fbo we can have multiple simultaneous ones(GL_ARB_draw_buffers, or multiple render targets in d3d speak). In case of onscreen rendering only one render target may be active from the d3d side, and in case of back/aux and pbuffer offscreen rendering we're limited to one target from the opengl side.
What has to be done in some way is to merge apply_fbo_state() with ActivateContext. My original idea was to select the primary target and the depthstencil buffer in ActivateContext similarly to the existing code, and have higher render targets set in the state manager. Unfortunately drivers do not like that, and we have to set all color and depth attachments of the fbo at once.
Also don't forget that a lot of function calls (especially in surface.c) still mess with glDrawBuffer themselves. For a lot of calls this is redundant.
Roderick
Am Sonntag, 25. November 2007 20:26:21 schrieb Roderick Colenbrander:
Also don't forget that a lot of function calls (especially in surface.c) still mess with glDrawBuffer themselves. For a lot of calls this is redundant.
Yes, most of them should be removed now that ActivateContext takes care of that. Some are needed, e.g. blt_to_texture_hwstretch switches to AUX0 / AUX1 to use it as a temporary buffer
The framebuffer setup should be merged with ActivateContext, but not by simply setting the framebuffer to 0. Currently there are two issues: the one you are seeing(the complaints from opengl), and multithreading issues.
Parts of the code call ActivateContext before setting the fbo: In this case you can get errors in glDrawBuffers. Other parts call the fbo setup before calling ActivateContext. In this case the app can crash after a rendering thread switch because no GL context is active.
Ah, ok, I think I've seen the threading issues as well.
The whole issue is pretty complex unfortunately. One issue is that we cannot select context and drawable separately for onscreen rendering, and pbuffer offscreen rendering. In the case of fbos, the drawable can be set without changing the context.
Another issue is that ActivateContext deals with one buffer only, but with fbo we can have multiple simultaneous ones(GL_ARB_draw_buffers, or multiple render targets in d3d speak). In case of onscreen rendering only one render target may be active from the d3d side, and in case of back/aux and pbuffer offscreen rendering we're limited to one target from the opengl side.
What has to be done in some way is to merge apply_fbo_state() with ActivateContext. My original idea was to select the primary target and the depthstencil buffer in ActivateContext similarly to the existing code, and have higher render targets set in the state manager. Unfortunately drivers do not like that, and we have to set all color and depth attachments of the fbo at once.
Thanks for the info.
- Allan
Here's my second attempt at this, this time merging FBO draw buffer selection into ActivateContext. Could you please review it? Hopefully I'm going in the right direction. The first patch moves the call to ActivateContext before the GL calls that setup the blit. This is needed so the second patch doesn't break stretch_rect_fbo.
The second patch moves FBO draw buffer selection to ActivateContext. For onscreen rendering, not much changes except binding framebuffer 0. For offscreen rendering, the behavior depends on the specified context usage. For BLIT and RESOURCE_LOAD, the target surface is attached to dst_fbo and used as the draw buffer. For DRAWPRIM and CLEAR, I simply call apply_fbo_state. This assumes that DRAWPRIM and CLEAR will always be used with device->render_targets[0].
With these patches, the visual tests in ddraw/d3d8/d3d9 mostly succeed with FBO offscreen rendering. The newly added alpha blending test still fails in ddraw, but it still does better than without the patches.
- Allan
Am Samstag, 15. Dezember 2007 19:19:25 schrieb Allan Tong:
Sorry for the late reply, your mail got stuck in my messy inbox :-(
The second patch moves FBO draw buffer selection to ActivateContext. For onscreen rendering, not much changes except binding framebuffer 0. For offscreen rendering, the behavior depends on the specified context usage. For BLIT and RESOURCE_LOAD, the target surface is attached to dst_fbo and used as the draw buffer. For DRAWPRIM and CLEAR, I simply call apply_fbo_state. This assumes that DRAWPRIM and CLEAR will always be used with device->render_targets[0].
There is a problem with GL_COLOR_ATTACHMENT1_EXT and higher, and the depth attachment. You're only applying the first color attachment, but that can lead to incomplete fbos, if for example the render target and depth stencil are changed. If you're only reapplying the first color attachment, you can get a size mismatch, which makes the driver pretty angry and can slow everything down.
You don't have to apply the fbo state in CTXUSAGE_RESOURCELOAD in theory. Resourceload never draws anything, nor does it read from the framebuffer, so it should be fine with any drawable / framebuffer.
On Dec 17, 2007 6:18 AM, Stefan Dösinger stefan@codeweavers.com wrote:
Am Samstag, 15. Dezember 2007 19:19:25 schrieb Allan Tong:
Sorry for the late reply, your mail got stuck in my messy inbox :-(
No problem.
The second patch moves FBO draw buffer selection to ActivateContext. For onscreen rendering, not much changes except binding framebuffer 0. For offscreen rendering, the behavior depends on the specified context usage. For BLIT and RESOURCE_LOAD, the target surface is attached to dst_fbo and used as the draw buffer. For DRAWPRIM and CLEAR, I simply call apply_fbo_state. This assumes that DRAWPRIM and CLEAR will always be used with device->render_targets[0].
There is a problem with GL_COLOR_ATTACHMENT1_EXT and higher, and the depth attachment. You're only applying the first color attachment, but that can lead to incomplete fbos, if for example the render target and depth stencil are changed. If you're only reapplying the first color attachment, you can get a size mismatch, which makes the driver pretty angry and can slow everything down.
As far as I can tell, the only thing that's ever attached to device->dst_fbo is the first color attachment, so I wouldn't think there's any chance for a size mismatch. Do I still need to unset the depth attachment or the higher color attachments? I'm assuming there's no reason to have a depth attachment for a blit operation.
You don't have to apply the fbo state in CTXUSAGE_RESOURCELOAD in theory. Resourceload never draws anything, nor does it read from the framebuffer, so it should be fine with any drawable / framebuffer.
I left it in since some places call ActivateContext using CTXUSAGE_RESOURCELOAD but with something other than device->lastActiveRenderTarget, e.g. SetRenderTarget. I wasn't sure if there is any code that's relying on the draw buffer being set by these calls.
- Allan
On 17/12/2007, Allan Tong actong88@gmail.com wrote:
The second patch moves FBO draw buffer selection to ActivateContext. For onscreen rendering, not much changes except binding framebuffer 0. For offscreen rendering, the behavior depends on the specified context usage. For BLIT and RESOURCE_LOAD, the target surface is attached to dst_fbo and used as the draw buffer. For DRAWPRIM and CLEAR, I simply call apply_fbo_state. This assumes that DRAWPRIM and CLEAR will always be used with device->render_targets[0].
There is a problem with GL_COLOR_ATTACHMENT1_EXT and higher, and the depth attachment. You're only applying the first color attachment, but that can lead to incomplete fbos, if for example the render target and depth stencil are changed. If you're only reapplying the first color attachment, you can get a size mismatch, which makes the driver pretty angry and can slow everything down.
As far as I can tell, the only thing that's ever attached to device->dst_fbo is the first color attachment, so I wouldn't think there's any chance for a size mismatch. Do I still need to unset the depth attachment or the higher color attachments? I'm assuming there's no reason to have a depth attachment for a blit operation.
Currently src_fbo and dst_fbo are only used for doing blits between surfaces in stretch_rect_fbo() and a colorfill in color_fill_fbo(), so there will only ever be something attached to GL_COLOR_ATTACHMENT0_EXT.
-
Allan Tong -
H. Verbeet -
Roderick Colenbrander -
Stefan Dösinger