Re: [PATCH 2/6] wined3d: Pass the volume texture level to the client libraries.
On 17 April 2014 15:47, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
The goal is to remove volumes and surfaces from the wined3d public API. This extra information is needed for the transition. We got rid of this not too long ago, adding it back doesn't sound very attractive. If you really need a way to find out a volume's level, I'd prefer wined3d_volume_get_level() instead, mostly because it doesn't need changes in every D3D library. I'm not convinced this is really necessary though, or that starting in d3d9 as opposed to wined3d with this is necessarily the right approach.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2014-04-18 13:08, schrieb Henri Verbeet:
We got rid of this not too long ago, adding it back doesn't sound very attractive. If you really need a way to find out a volume's level, I'd prefer wined3d_volume_get_level() instead, mostly because it doesn't need changes in every D3D library. As mentioned passing the information from wined3d is a temporary change only. Patch 6 removes it from d3d9. Once the same changes have been made to d3d8 the creation callback is gone.
I guess I could avoid the temporary change by reordering the patches.
I'm not convinced this is really necessary though, or that starting in d3d9 as opposed to wined3d with this is necessarily the right approach. I prefer to remove as many calls to surfaces and volumes as possible before changing them not to be resources. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJTUU4QAAoJEN0/YqbEcdMwr3YP/3ORrlipM1igNGQv9TFSJhg2 Q4hNUzEKNbpB94fsbI+HAbtX8LyvH5rC5R17aHTURrk3qo7PknfOrKE74T+jeD50 lCHR3YvJakRabjuvUn99bD6CAMMGaUVSr1tWIyoeplzoOoZ8DidnrnifubVCMaWl TIjcVW8elMwbi25tZkya9qELsb1BL53HTWTWlc/9L6a2OkiEqtX9t+/e+gliQE6X uB4PVXgDKN5D1Bhr4G3442SnHLcldbdy/oP/RbSKlGZaL+0Y3wu2CWtLU9XxvfCq bggGqpIWvOjM51zxj0kb1dSS/xQ4lOlhPUbE+UQVS8lJfz6jnwgvPB94plKEvJQ4 vrRUtG2wCdLmYPQ7kgI+6xb2QJLB+fMdf2z6h8l30/cXvkTdUdO4ifzZTMC15wJs PBRx7ALtbyh3rvPi4B9TnTcdC6KkgyciAGH2lfRA3yVmz84qnz0/qKs+WE6dZbxp mPI84DfH2KYwXZ1YxHoZHwj7/P58WYFcNRXGOi5FivF6Q2rXAbTg3+fTwxyko9Lr cEexXHxD39E7qTtxWDtRIPHrbsXUJFZ3mR3GAu6/6ZhqbeHYM8Fc4Em9vVDXvKf9 1VY+vX+MC8mH6TETP78riSekJYLXbPEIqil4yAYPBOEr6vO7PZqrGUxFTd+NUNM3 FxPWn41kSD0FXfmQTdqw =Vi8n -----END PGP SIGNATURE-----
On 18 April 2014 18:08, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
I'm not convinced this is really necessary though, or that starting in d3d9 as opposed to wined3d with this is necessarily the right approach. I prefer to remove as many calls to surfaces and volumes as possible before changing them not to be resources. I don't care about that ordering as much by itself. I do think that it makes more sense to approach this as removing surfaces and volumes from the public wined3d API than the approach you're taking here. I.e., instead of e.g. forwarding d3d9_volume_LockBox() to d3d9_texture_3d_LockBox() that then still does a wined3d_volume_map(), I think it makes more sense to introduce wined3d_texture_map(), get rid of wined3d_volume_map() and wined3d_surface_map(), and then let the rest just fall out as a logical consequence. Related to that, I think surfaces are the more interesting case to start with, because of things like e.g. wined3d_device_set_render_target(), and perhaps more importantly wined3d_device_get_render_target().
Am 18.04.2014 um 18:58 schrieb Henri Verbeet <hverbeet(a)gmail.com>:
I don't care about that ordering as much by itself. I do think that it makes more sense to approach this as removing surfaces and volumes from the public wined3d API than the approach you're taking here. I.e., instead of e.g. forwarding d3d9_volume_LockBox() to d3d9_texture_3d_LockBox() that then still does a wined3d_volume_map(), I think it makes more sense to introduce wined3d_texture_map(), get rid of wined3d_volume_map() and wined3d_surface_map(), and then let the rest just fall out as a logical consequence. Related to that, I think surfaces are the more interesting case to start with, because of things like e.g. wined3d_device_set_render_target(), and perhaps more importantly wined3d_device_get_render_target(). Merging surface / volume map in texture_map doesn’t work well without having a common (internal) sub resource interface that the texture can call. Unless of course I do a switch(texture_type) and forward this to the separate surface_map and volume_map internally.
On 18 April 2014 19:28, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
Merging surface / volume map in texture_map doesn’t work well without having a common (internal) sub resource interface that the texture can call. Unless of course I do a switch(texture_type) and forward this to the separate surface_map and volume_map internally.
There's struct wined3d_texture_ops if there are no better options, but yes, this may imply doing work to unify wined3d_surface_map() and wined3d_volume_map() first. That was just an example though, the basic point is that moving wined3d_volume calls from the d3d9 volume to the d3d9 volume texture doesn't get us much closer to getting rid of wined3d_volume in the public wined3d API. I did warn that this was non-trivial to get right.
Am 18.04.2014 um 20:03 schrieb Henri Verbeet <hverbeet(a)gmail.com>:
There's struct wined3d_texture_ops if there are no better options, but yes, this may imply doing work to unify wined3d_surface_map() and wined3d_volume_map() first. That was just an example though, the basic point is that moving wined3d_volume calls from the d3d9 volume to the d3d9 volume texture doesn't get us much closer to getting rid of wined3d_volume in the public wined3d API. I did warn that this was non-trivial to get right. We could go back to http://www.winehq.org/pipermail/wine-patches/2014-February/130248.html and merge resource management and resource mapping in a sub resource structure while keeping the resource structure around as long as needed. The main difference I’d suggest is not deriving wined3d_sub_resource from wined3d_resource.
I’d prefer, but that’s not a requirement, to remove the get_resource_desc calls on surfaces and volumes before doing that. I realize that not calling those functions doesn’t make the interface go away (the only way to do that is to remove resource_from_surface), but it avoids exposing the to-be-removed resource data outside of wined3d. In terms of where we want to go wrt surfaces/volumes, subresources and resources: Surfaces and volumes would be responsible for storing and loading data in the GL texture. Surfaces also handle DIB sections for GetDC. Subresources take care of location management (the main motivation: Offscreen ORM can only load one mipmap sublevel into WINED3D_LOCATION_DRAWABLE). They take care of mapping PBOs and returning the correct pointer offset (motivation: user memory and DIBs are set per subresource). There’s some wiggle room on this point though. Subresources store the container, which is a wined3d_resource *. Textures (or rather, resources) take care of allocating textures, sysmem and PBOs. subresources can request the allocation and freeing of sysmem from their container. Textures will be resources, surfaces/volumes will be subresources. Buffers will be both, and they will be their own container. Resources take care of handling WINED3D_MAP_DISCARD and allocating new sysmem / pbos if necessary. While textures will allocate PBOs, they will allocate individual PBOs for each subresource because subresources can be mapped individually. In situations where GL_ARB_buffer_storage is available, and creating a an always-mapped PBO is feasible we can create one PBO for the entire texture and pre-map it at creation time or the first map. The main thing where I see two options with advantages / disadvantages: Do we store DIB and User pointer in the subresource or the surface: Storing them in the surface seems like the right place at first sight because they’re surface specific. But subresources need code to copy data between buffer and sysmem, and this code can be reused for DIBs and user memory as well. In that case we’d only call volume / surface specific code for loading textures, which means that code can assume a GL context is available. In my CS code surfaces allocate DIBs, but the DIB pointer is stored in struct wined3d_sub_resource. Somewhat related, I wonder if the hSection parameter of CreateDIBSection can be used to wrap a DIB section around our HeapAlloc’ed sysmem.
participants (2)
-
Henri Verbeet -
Stefan Dösinger