On Wed, Apr 20, 2016 at 10:34 AM, Henri Verbeet hverbeet@gmail.com wrote:
Well, yes. But it's not up to surface_download_data() to decide whether to use GetTexImage() or ReadPixels(). surface_load_sysmem() should decide if it's more appropriate to use surface_download_data() or read_from_framebuffer().
That said, I'm not sure read_from_framebuffer() is necessarily an improvement in terms of (overall) performance. True, GetTexImage() reads more data than it needs to, but I don't think it's a given that that's much more expensive than reading a smaller amount of data unless the texture is particularly large. In cases where you're going to access the other sub-resources afterwards anyway it may even be an improvement, provided resource locations etc. are properly updated. On the other hand, read_from_framebuffer() calls context_apply_blit_state(), which does a lot of state invalidation. If multiple of those are batched together that may be ok (but in that case perhaps surface_download_data() would be too), but if they're interleaved with draws it may not be an improvement.
I.e., it would be a lot easier to justify this if you could show an application where it helps.
I don't have any application to show that this helps. I'll drop the patch.
Is patch 121528 ok in its current form? It doesn't feel like a good idea messing with other surfaces in surface_download_data().