On Wed, Oct 24, 2018 at 9:53 AM Henri Verbeet hverbeet@gmail.com wrote:
Well, maybe. A few questions though:
Great questions, in fact. Replies inline.
- You had tests, right? Does this really make sense for textures as well?
I'm attaching one of the tests for reference but indeed I completely forgot to test textures :/ It turns out that D3DPOOL_SYSTEMMEM textures, as expected, aren't supposed to be usable in draws (with d3d8/d3d9, the draw returns E_FAIL, weirdly with ddraw it's EndScene() that fails). I made wined3daccess_from_d3dpool() also take a resource type parameter and only add GPU access in the buffer case. I think I like it better now but let me know if you see it otherwise.
- What is the conceptual model? I.e., after this series, what is the
difference between a D3DPOOL_SYSTEMMEM and a D3DPOOL_MANAGED resource?
There isn't much of a difference for buffers, admittedly. The docs suggest that resource updates might follow a slightly different policy between the two, which I'm not sure is really testable. Textures are going to stay as they were, which means, among other things, that we need to keep WINED3DUSAGE_SCRATCH around.
- Does it really make sense that D3DPOOL_MANAGED resources are
evicted on wined3d_device_evict_managed_resources(), but D3DPOOL_SYSTEMMEM resources are potentially kept on the GPU?
Good point, I updated the patches accordingly. From my understanding of the docs, I think the system memory copy of D3DPOOL_SYSTEMMEM buffers is always valid / current in d3d so native EvictManagedResources() can simply ignore those. Going further on that line of thought, I realized that we should probably PIN_SYSMEM those buffers and a new test confirms that to be the case.