On 6 April 2015 at 00:28, Stefan Dösinger stefan@codeweavers.com wrote:
+#define WINED3D_CKEY_REMOVE 0x80000000
It seems unfortunate to define part of the flags in include/wine/wined3d.h and part of the flags in dlls/wined3d/cs.c. You'd probably also want to mask out private flags in wined3d_cs_emit_set_color_key().
Ultimately though, why do you need this? Most of the CS ops operate on a wined3d_state structure, which is mirrored between the CS and the device, and (in theory) functions get passed the appropriate one depending on whether they're running from the CS or not. This one is different in that it operates on a texture, and anything that would need to read the color key would then need to either wait for the texture to become idle, or run from the CS itself. Perhaps that's ok, but it's more of a fundamental design decision than this commit makes it seem.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2015-04-07 um 12:17 schrieb Henri Verbeet:
On 6 April 2015 at 00:28, Stefan Dösinger stefan@codeweavers.com wrote:
+#define WINED3D_CKEY_REMOVE 0x80000000 +
It seems unfortunate to define part of the flags in include/wine/wined3d.h and part of the flags in dlls/wined3d/cs.c. You'd probably also want to mask out private flags in wined3d_cs_emit_set_color_key().
There are multiple ways to pass this information. The simplest one is an extra field in struct wined3d_cs_set_color_key, but that seems unfortunate because we have plenty of spare bits elsewhere. I could also define a separate op for unsetting the key. Whatever you prefer.
This one is different in that it operates on a texture, and anything that would need to read the color key would then need to either wait for the texture to become idle, or run from the CS itself.
The color key is currently read by the texture loading code, which is either called by draws (called through the CS) or by blits (which we want to call through the CS at some point). I don't see a clean way to move the read to the main thread.
What we could do is shadow the texture 0 color key in wined3d_state. In this case the command stream op would only be used for changing this mirrored value, and it would be emitted when texture 0 is changed or when texture 0's color key is changed(*). In this case we need a separate way to pass the key to blits (could be a blit op parameter, and passed around between functions), and wined3d_texture_load needs to somehow find out when to read the color key from the state structure.
Note that for GLSL / ARB fragment processing I'll handle color keying with discard / KIL in the shader and the color key becomes more or less a shader constant, and I'll keep track of changes to it with a new state.
The immediate requirement that prompted this change is that setting the key on the texture that's currently bound to texture 0 should invalidate WINED3D_RS_COLORKEYENABLE. The state invalidation must be done through the command stream or wait for all commands to be executed.
(*): Windows drivers disagree on color keying on textures 1-7. dx9+ GPUs apply the color key, at least my Radeon 9000 (dx8) does not, and doesn't seem to have circuitry for more than one key. I think we can safely keep color keying limited to texture 0.
On 7 April 2015 at 12:53, Stefan Dösinger stefandoesinger@gmail.com wrote:
This one is different in that it operates on a texture, and anything that would need to read the color key would then need to either wait for the texture to become idle, or run from the CS itself.
The color key is currently read by the texture loading code, which is either called by draws (called through the CS) or by blits (which we want to call through the CS at some point). I don't see a clean way to move the read to the main thread.
In principle it's probably ok if it can't be directly read/written by the main thread unless the texture is idle. (But note that this also affects e.g. preload calls.) It would probably be a good idea to make that explicit though. E.g. one approach could perhaps be something like this:
struct wined3d_texture { ... /* Explanation here. */ struct { struct wined3d_color_key dst_blt_color_key; ... } async; };
So that anything touching those fields is aware of what is and isn't safe to touch from the main thread.
I haven't looked at the async CS patchset in while now, but I gather that you're sending pretty much anything that might need a GL context at some point through the CS, supposedly because of issues with various proprietary drivers. It's probably no huge surprise that if that's the case, that justification doesn't fill me with great joy. Regardless, I'd like to at least keep it possible to do resource updates from threads other than the CS thread, at least until the performance trade-offs are well understood, particularly for applications that use D3D from multiple threads.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2015-04-08 um 14:07 schrieb Henri Verbeet:
struct wined3d_texture { ... /* Explanation here. */ struct { struct wined3d_color_key dst_blt_color_key; ... } async; };
I like this idea. There will be some things that will be accessed by both (e.g. a fence ULONG, possibly some things related to maps), those could go into their own struct. Some fields like surface flags need splitting
-
Henri Verbeet -
Stefan Dösinger