Re: [PATCH 1/5] wined3d: Don't bother about client storage in wined3d_surface_set_mem.

2013-10-04 Thread Henri Verbeet
On 4 October 2013 00:03, Stefan Dösinger ste...@codeweavers.com wrote:
 @@ -2883,10 +2886,6 @@ HRESULT CDECL wined3d_surface_set_mem(struct 
 wined3d_surface *surface, void *mem
  /* Now the surface memory is most up do date. Invalidate drawable 
 and texture. */
  surface_validate_location(surface, SFLAG_INSYSMEM);
  surface_invalidate_location(surface, ~SFLAG_INSYSMEM);
 -
 -/* For client textures OpenGL has to be notified. */
 -if (surface-flags  SFLAG_CLIENT)
 -surface_release_client_storage(surface);
  }
  else if (surface-flags  SFLAG_USERPTR)
  {
 @@ -2899,9 +2898,6 @@ HRESULT CDECL wined3d_surface_set_mem(struct 
 wined3d_surface *surface, void *mem
  surface-resource.allocatedMemory = NULL;
  surface-flags = ~(SFLAG_USERPTR | SFLAG_INSYSMEM);

 -if (surface-flags  SFLAG_CLIENT)
 -surface_release_client_storage(surface);
 -
  surface_prepare_system_memory(surface);
  }

What makes this safe, and why is it needed?




Re: [PATCH 1/5] wined3d: Don't bother about client storage in wined3d_surface_set_mem.

2013-10-04 Thread Henri Verbeet
On 4 October 2013 15:02, Stefan Dösinger stefandoesin...@gmail.com wrote:
 Client storage only applies to GL textures, which won't be created for sysmem 
 surfaces.

I don't think that's necessarily true at the moment. In particular,
ddraw blits can in principle cause a texture to be created for sysmem
surfaces. There might be restrictions in the ddraw API that prevent
this from happening in practice, but even if there are, we certainly
don't enforce them.




Re: [PATCH 1/5] wined3d: Don't bother about client storage in wined3d_surface_set_mem.

2013-10-04 Thread Stefan Dösinger

Am 04.10.2013 um 15:28 schrieb Henri Verbeet hverb...@gmail.com:

 On 4 October 2013 15:02, Stefan Dösinger stefandoesin...@gmail.com wrote:
 Client storage only applies to GL textures, which won't be created for 
 sysmem surfaces.
 
 I don't think that's necessarily true at the moment. In particular,
 ddraw blits can in principle cause a texture to be created for sysmem
 surfaces. There might be restrictions in the ddraw API that prevent
 this from happening in practice, but even if there are, we certainly
 don't enforce them.
No codepath in wined3d_surface_blt will attempt to load a sysmem surface into a 
texture. fbo_blit_supported returns FALSE if src or destination are in sysmem, 
and so do arbfp_blit_supported and surface_blt_special. Color fills will go to 
a cpu side clear because of the INSYSMEM optimization. (This optimization is 
needed for a few other things to work correctly, but that's a different patch.)





Re: [PATCH 1/5] wined3d: Don't bother about client storage in wined3d_surface_set_mem.

2013-10-04 Thread Stefan Dösinger
Am 04.10.2013 um 15:51 schrieb Stefan Dösinger stefandoesin...@gmail.com:
 No codepath in wined3d_surface_blt will attempt to load a sysmem surface into 
 a texture. fbo_blit_supported returns FALSE if src or destination are in 
 sysmem, and so do arbfp_blit_supported and surface_blt_special. Color fills 
 will go to a cpu side clear because of the INSYSMEM optimization. (This 
 optimization is needed for a few other things to work correctly, but that's a 
 different patch.)
Fwiw, the other patches are independent of this patch.





Re: [PATCH 1/5] wined3d: Don't bother about client storage in wined3d_surface_set_mem.

2013-10-04 Thread Henri Verbeet
On 4 October 2013 15:51, Stefan Dösinger stefandoesin...@gmail.com wrote:
 No codepath in wined3d_surface_blt will attempt to load a sysmem surface into 
 a texture. fbo_blit_supported returns FALSE if src or destination are in 
 sysmem, and so do arbfp_blit_supported and surface_blt_special. Color fills 
 will go to a cpu side clear because of the INSYSMEM optimization. (This 
 optimization is needed for a few other things to work correctly, but that's a 
 different patch.)

I guess that makes it ok in practice, but I'd still feel happier about
this kind of patch if we actually enforced resource access flags
first. (At which point you could also just check the access flags
instead of the pool.)




Re: [PATCH 1/5] wined3d: Don't bother about client storage in wined3d_surface_set_mem.

2013-10-04 Thread Stefan Dösinger

Am 04.10.2013 um 17:15 schrieb Henri Verbeet hverb...@gmail.com:
 I guess that makes it ok in practice, but I'd still feel happier about
 this kind of patch if we actually enforced resource access flags
 first. (At which point you could also just check the access flags
 instead of the pool.)
My plan is to enforce this in resource_load_location similar to how it's 
currently done for volumes. That'd be at the end of my surface cleanup patchset 
though, and it'll need some additional work (downloading a texture to a bigger 
sysmem surface to handle get_front_buffer_data).

I think my surface patches should work ok without the assumption that client 
storage and user memory are mutually exclusive, so feel free to skip this patch 
for now. The other 4 patches in this series should work ok. Some later patches 
in the big patchset need minor adjustments though.