Re: wined3d: Track when the FBO is dirty. (try 3)

[Henri Verbeet]

On 7 April 2011 18:51, Adam Martinson  wrote:
> -
>        context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
> -
> -
>        if (rt_count == device->adapter->gl_info.limits.buffers &&
> -
>            rts == device->render_targets &&
> -
>            depth_stencil == device->depth_stencil)
> -
>        {
> -
>            if (context->device_fbo != NULL)
> -
>            {
> -
>                context_apply_device_fbo(context);
> -
>            }
> -
>            else
> -
>            {
> -
>                context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
> -
>                context->device_fbo = context->current_fbo;
> -
>            }
> -
>        }
> -
>        else
> -
>        {
> -
>            context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
> -
>        }

That's not very pretty.
> @@ -6709,6 +6721,7 @@ void device_resource_released(struct IWineD3DDeviceImpl *device, struct wined3d_
>                      {
>                          ERR("Surface %p is still in use as render target %u.\n", surface, i);
>                          device->render_targets[i] = NULL;
> -
>                    fbo_dirty = TRUE;
>                }
>            }
>
> @@ -6716,7 +6729,10 @@ void device_resource_released(struct IWineD3DDeviceImpl *device, struct wined3d_
>                  {
>                      ERR("Surface %p is still in use as depth/stencil buffer.\n", surface);
>                      device->depth_stencil = NULL;
> -
>                fbo_dirty = TRUE;

That code should never execute, hence the ERR.

wined3d: Track when the FBO is dirty. (try 3)

[Adam Martinson]

On 04/08/2011 08:50 AM, Henri Verbeet wrote:
> On 7 April 2011 18:51, Adam Martinson  wrote:
> > -
> >        context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
> > -
> > -
> >        if (rt_count == device->adapter->gl_info.limits.buffers&&
> > -
> >            rts == device->render_targets&&
> > -
> >            depth_stencil == device->depth_stencil)
> > -
> >        {
> > -
> >            if (context->device_fbo != NULL)
> > -
> >            {
> > -
> >                context_apply_device_fbo(context);
> > -
> >            }
> > -
> >            else
> > -
> >            {
> > -
> >                context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
> > -
> >                context->device_fbo = context->current_fbo;
> > -
> >            }
> > -
> >        }
> > -
> >        else
> > -
> >        {
> > -
> >            context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
> > -
> >        }
>
> That's not very pretty.

If you mean comparing rt_count/rts/depth_stencil vs the device, the 
other option I see is adding an extra argument to 
device_clear_render_targets() and context_apply_clear_state() to tell us 
if it's from a blit or not.  If you prefer doing it that way I can 
change it.
If you're referring to something else... you'll have to be a bit more 
specific.
> > @@ -6709,6 +6721,7 @@ void device_resource_released(struct IWineD3DDeviceImpl *device, struct wined3d_
> >                       {
> >                           ERR("Surface %p is still in use as render target %u.\n", surface, i);
> >                           device->render_targets[i] = NULL;
> > -
> >                    fbo_dirty = TRUE;
> >                 }
> >             }
> >
> > @@ -6716,7 +6729,10 @@ void device_resource_released(struct IWineD3DDeviceImpl *device, struct wined3d_
> >                   {
> >                       ERR("Surface %p is still in use as depth/stencil buffer.\n", surface);
> >                       device->depth_stencil = NULL;
> > -
> >                fbo_dirty = TRUE;
>
> That code should never execute, hence the ERR.

I understand that, I only did that for completeness; if either 
device::render_targets[] or device::depth_stencil changes then the FBO 
is dirty.  If you prefer to just call IWineD3DDeviceImpl_MarkFBODirty() 
there instead of using the variable, that's easy enough to change.  If 
you think we should not worry about it I can remove that part 
altogether.  Please let me know your preference.

wined3d: Track when the FBO is dirty. (try 3)

[Henri Verbeet]

Actually, I took a look at the benchmark you said this is supposed to
help. It doesn't seem to use offscreen rendering at all.

wined3d: Track when the FBO is dirty. (try 3)

[Adam Martinson]

On 04/11/2011 05:25 PM, Henri Verbeet wrote:
> Actually, I took a look at the benchmark you said this is supposed to
> help. It doesn't seem to use offscreen rendering at all.

I re-benched and I can confirm; context_apply_fbo_state() is no longer a 
significant bottleneck in the batch size tests.  However, this is a 
relatively recent change, in the past few weeks at most.  On the other 
3DMark06 tests that still do use offscreen rendering it's noticeable 
though, eg in HDR1/Canyon Flight it takes context_apply_fbo_state() from 
4.25% => 1.36% of wined3d.  Attached is the rebased version.