wined3d performance patches

Henri Verbeet hverbeet at gmail.com
Tue Jun 14 07:20:12 CDT 2011


I only looked over about half of this, doesn't look too crazy. You do
have trailing spaces in a couple of places though. Here are some
comments:

> Subject: [PATCH 04/14] wined3d: Don't set FBO attachment filtering to GL_NEAREST
As far as I'm concerned you can just submit this. I was going to do
this myself, looks like you got there first.

> Subject: [PATCH 05/14] wined3d: Move FBO application into a state handler
This needs an update to debug_d3dstate() as well.

> +static BOOL surface_is_framebuffer(struct wined3d_surface *surface)
> +{
> +    struct wined3d_device *device = surface->resource.device;
> +    unsigned int i;
> +    const struct wined3d_gl_info *gl_info = &device->adapter->gl_info;
> +
> +    if (surface == device->fb.depth_stencil) return TRUE;
> +    if (!device->fb.render_targets) return FALSE;
> +    for (i = 0; i < gl_info->limits.buffers; i++)
> +    {
> +        if (surface == device->fb.render_targets[i]) return TRUE;
> +    }
> +    return FALSE;
> +}
I don't know how much this actually hurts, but we could introduce
something along the lines of "SFLAG_ACTIVE_RT" to keep track of this.
We may have to worry about surfaces being bound to more than one RT
attachment point, in which case we'd need a count instead of a flag
though.

> @@ -1913,6 +1928,10 @@ void surface_set_texture_name(struct wined3d_surface *surface, GLuint new_name,
> ·
>      *name = new_name;
>      surface_force_reload(surface);
> +    if (surface_is_framebuffer(surface))
> +    {
> +        IWineD3DDeviceImpl_MarkStateDirty(surface->resource.device, STATE_FRAMEBUFFER);
> +    }
>  }
> ·
>  void surface_set_texture_target(struct wined3d_surface *surface, GLenum target)
> @@ -2317,6 +2336,10 @@ static void surface_allocate_surface(struct wined3d_surface *surface, const stru
>          checkGLcall("glPixelStorei(GL_UNPACK_CLIENT_STORAGE_APPLE, GL_TRUE)");
>      }
>      LEAVE_GL();
> +    if (surface_is_framebuffer(surface))
> +    {
> +        IWineD3DDeviceImpl_MarkStateDirty(surface->resource.device, STATE_FRAMEBUFFER);
> +    }
>  }
What are these for?

You may also have to handle an active RT getting unloaded, though I'm
not entirely sure if that's allowed or not.

> +    BOOL all_samplers_mapped = TRUE;
...
> +            all_samplers_mapped = FALSE;
This looks unused.

> +    for(i = 0; i < swapchain->presentParms.BackBufferCount; i++)
Style.

> Subject: [PATCH 09/14] wined3d: Make render target dirtification in draws cheaper
I'm not sure about this one. My first impression is that it looks
fragile. The rt_location_flags setup makes assumptions about
load_location() internals, which will likely change at some point. In
particular, now that most of the location management goes through the
appropriate functions instead of messing with the flags themselves,
I've been thinking about getting rid of the texture == drawable stuff
for FBOs, and instead just making texture <-> drawable loads a nop in
load_location(). At the very least this would allow the check to
always be against SFLAG_INDRAWABLE instead of
surface->rt_location_flags, which IMHO looks more sane. I wonder if
the speedup is mostly for load_location(), modify_location() or both
though? Maybe we can improve those functions themselves.



More information about the wine-devel mailing list