This reduces the diff with the Wine bundled version.
It should also make it clearer that these are Wine headers that should ideally be changed upstream first and then re-imported.
From: Alexandre Julliard julliard@winehq.org
This avoids having to change the code when importing into Wine. --- include/private/{ => wine}/list.h | 0 libs/vkd3d-shader/vkd3d_shader_private.h | 2 +- libs/vkd3d/vkd3d_private.h | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename include/private/{ => wine}/list.h (100%)
diff --git a/include/private/list.h b/include/private/wine/list.h similarity index 100% rename from include/private/list.h rename to include/private/wine/list.h diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index aca5606b..371fd62f 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -49,7 +49,7 @@ #include "vkd3d_common.h" #include "vkd3d_memory.h" #include "vkd3d_shader.h" -#include "list.h" +#include "wine/list.h"
#include <assert.h> #include <inttypes.h> diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index c9b70d9c..ccffb5de 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -31,7 +31,7 @@ #include "vkd3d_blob.h" #include "vkd3d_memory.h" #include "vkd3d_utf8.h" -#include "list.h" +#include "wine/list.h" #include "rbtree.h"
#include "vkd3d.h"
From: Alexandre Julliard julliard@winehq.org
This avoids having to change the code when importing into Wine. --- include/private/{ => wine}/rbtree.h | 0 libs/vkd3d-shader/hlsl.h | 2 +- libs/vkd3d-shader/preproc.h | 2 +- libs/vkd3d-shader/spirv.c | 2 +- libs/vkd3d/vkd3d_private.h | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename include/private/{ => wine}/rbtree.h (100%)
diff --git a/include/private/rbtree.h b/include/private/wine/rbtree.h similarity index 100% rename from include/private/rbtree.h rename to include/private/wine/rbtree.h diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 794749aa..26fa9c92 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -21,7 +21,7 @@ #define __VKD3D_SHADER_HLSL_H
#include "vkd3d_shader_private.h" -#include "rbtree.h" +#include "wine/rbtree.h" #include "vkd3d_d3dcommon.h" #include "vkd3d_d3dx9shader.h" #include "sm4.h" diff --git a/libs/vkd3d-shader/preproc.h b/libs/vkd3d-shader/preproc.h index e1cb75e1..4860cf5f 100644 --- a/libs/vkd3d-shader/preproc.h +++ b/libs/vkd3d-shader/preproc.h @@ -22,7 +22,7 @@ #define __VKD3D_SHADER_PREPROC_H
#include "vkd3d_shader_private.h" -#include "rbtree.h" +#include "wine/rbtree.h"
struct preproc_if_state { diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 04dd133f..7c46cda1 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -18,7 +18,7 @@ */
#include "vkd3d_shader_private.h" -#include "rbtree.h" +#include "wine/rbtree.h"
#include <stdarg.h> #include <stdio.h> diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index ccffb5de..dbf38279 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -32,7 +32,7 @@ #include "vkd3d_memory.h" #include "vkd3d_utf8.h" #include "wine/list.h" -#include "rbtree.h" +#include "wine/rbtree.h"
#include "vkd3d.h" #include "vkd3d_shader.h"
From: Alexandre Julliard julliard@winehq.org
--- include/private/wine/rbtree.h | 54 ++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 7 deletions(-)
diff --git a/include/private/wine/rbtree.h b/include/private/wine/rbtree.h index b5d38bca..81367f3f 100644 --- a/include/private/wine/rbtree.h +++ b/include/private/wine/rbtree.h @@ -101,6 +101,13 @@ static inline struct rb_entry *rb_head(struct rb_entry *iter) return iter; }
+static inline struct rb_entry *rb_tail(struct rb_entry *iter) +{ + if (!iter) return NULL; + while (iter->right) iter = iter->right; + return iter; +} + static inline struct rb_entry *rb_next(struct rb_entry *iter) { if (iter->right) return rb_head(iter->right); @@ -108,6 +115,13 @@ static inline struct rb_entry *rb_next(struct rb_entry *iter) return iter->parent; }
+static inline struct rb_entry *rb_prev(struct rb_entry *iter) +{ + if (iter->left) return rb_tail(iter->left); + while (iter->parent && iter->parent->left == iter) iter = iter->parent; + return iter->parent; +} + static inline struct rb_entry *rb_postorder_head(struct rb_entry *iter) { if (!iter) return NULL; @@ -145,7 +159,7 @@ static inline struct rb_entry *rb_postorder_next(struct rb_entry *iter) /* iterate through the tree using a tree entry and postorder, making it safe to free the entry */ #define RB_FOR_EACH_ENTRY_DESTRUCTOR(elem, elem2, tree, type, field) \ for ((elem) = RB_ENTRY_VALUE(rb_postorder_head((tree)->root), type, field); \ - (elem) != WINE_RB_ENTRY_VALUE(0, type, field) \ + (elem) != RB_ENTRY_VALUE(0, type, field) \ && (((elem2) = RB_ENTRY_VALUE(rb_postorder_next(&(elem)->field), type, field)) || 1); \ (elem) = (elem2))
@@ -168,18 +182,13 @@ static inline void rb_for_each_entry(struct rb_tree *tree, rb_traverse_func *cal RB_FOR_EACH(iter, tree) callback(iter, context); }
-static inline void rb_clear(struct rb_tree *tree, rb_traverse_func *callback, void *context) +static inline void rb_destroy(struct rb_tree *tree, rb_traverse_func *callback, void *context) { /* Note that we use postorder here because the callback will likely free the entry. */ if (callback) rb_postorder(tree, callback, context); tree->root = NULL; }
-static inline void rb_destroy(struct rb_tree *tree, rb_traverse_func *callback, void *context) -{ - rb_clear(tree, callback, context); -} - static inline struct rb_entry *rb_get(const struct rb_tree *tree, const void *key) { struct rb_entry *entry = tree->root; @@ -375,4 +384,35 @@ static inline void rb_remove_key(struct rb_tree *tree, const void *key) if (entry) rb_remove(tree, entry); }
+static inline void rb_replace(struct rb_tree *tree, struct rb_entry *dst, struct rb_entry *src) +{ + if (!(src->parent = dst->parent)) + tree->root = src; + else if (dst->parent->left == dst) + dst->parent->left = src; + else + dst->parent->right = src; + + if ((src->left = dst->left)) + src->left->parent = src; + if ((src->right = dst->right)) + src->right->parent = src; + src->flags = dst->flags; +} + +/* old names for backwards compatibility */ +#define wine_rb_entry rb_entry +#define wine_rb_tree rb_tree +#define wine_rb_init rb_init +#define wine_rb_for_each_entry rb_for_each_entry +#define wine_rb_destroy rb_destroy +#define wine_rb_get rb_get +#define wine_rb_put rb_put +#define wine_rb_remove rb_remove +#define wine_rb_remove_key rb_remove_key +#define wine_rb_replace rb_replace +#define WINE_RB_ENTRY_VALUE RB_ENTRY_VALUE +#define WINE_RB_FOR_EACH_ENTRY RB_FOR_EACH_ENTRY +#define WINE_RB_FOR_EACH_ENTRY_DESTRUCTOR RB_FOR_EACH_ENTRY_DESTRUCTOR + #endif /* __WINE_WINE_RBTREE_H */
It should also make it clearer that these are Wine headers that should ideally be changed upstream first and then re-imported.
That's not how these were intended though. These were copied from Wine because it was convenient, but we were never particularly concerned about these diverging from Wine; ultimately lists and RB-trees aren't that complicated. Arguably these should have been named vkd3d_list.h and vkd3d_rbtree.h, similar to the other headers in private/ to make that clearer from the start.
I'm not convinced that this is something that should change either; being much more independent from Wine than e.g. wined3d is was an explicit design choice when vkd3d was created. We'd like e.g. the shader compiler to be useful to native Linux applications, and we'd like people to be able to contribute to that without being particularly concerned about e.g. building Wine.
I'm not convinced that this is something that should change either; being much more independent from Wine than e.g. wined3d is was an explicit design choice when vkd3d was created. We'd like e.g. the shader compiler to be useful to native Linux applications, and we'd like people to be able to contribute to that without being particularly concerned about e.g. building Wine.
In theory, sure. In practice the only user and contributor is Wine, so it seems to make more sense to prioritize that usage, even at the cost of slightly complicating things for some hypothetical non-Wine contributor.
In theory, sure. In practice the only user and contributor is Wine, so it seems to make more sense to prioritize that usage, even at the cost of slightly complicating things for some hypothetical non-Wine contributor.
I don't agree. Perhaps more important though, it's not quite clear to me what issue we're trying to avoid on the Wine side by replacing the vkd3d copies of these with the Wine copies. Is this purely about modules like e.g. wined3d ending up with two versions of e.g. list_init() in the same module, or is this about something else? I'd be happy to rename the vkd3d versions of these if that helps.
I don't agree. Perhaps more important though, it's not quite clear to me what issue we're trying to avoid on the Wine side by replacing the vkd3d copies of these with the Wine copies. Is this purely about modules like e.g. wined3d ending up with two versions of e.g. list_init() in the same module, or is this about something else? I'd be happy to rename the vkd3d versions of these if that helps.
It's mostly to avoid having to import more code than necessary. And of course for people who work on both vkd3d and Wine, using the same helpers makes life easier.
I agree that it's no big deal for list.h. I was going to look into sharing more things, like wine/debug.h or the idl headers, but I assume that's not desirable either?
It's mostly to avoid having to import more code than necessary. And of course for people who work on both vkd3d and Wine, using the same helpers makes life easier.
I agree that it's no big deal for list.h. I was going to look into sharing more things, like wine/debug.h or the idl headers, but I assume that's not desirable either?
It would depend a bit on the specifics, but most likely not, no. As a general principle, I think vkd3d would like to avoid cases where we'd tell people things like "Please fix this in Wine instead" or "You can't change that internal header because it would break Wine".
I know @zfigura has opinions on this subject as well, and I don't want to get too far ahead of those, but to touch a little on the subject of non-Wine users/contributors, I think it's important enough to attempt to attract those that I'd tell them "vkd3d will go out of its way to accommodate your use-cases, even if that makes things slightly harder for us as Wine developers." Going by what you wrote above, I think that's the opposite of how you see things, which is a bit unfortunate. :-\
It is indeed true that vkd3d doesn't currently have a whole lot of non-Wine users/contributors, but we do in fact have a number of contributors with no commits in Wine or much fewer commits in Wine than in vkd3d. We also haven't made much of an effort to approach potential users of vkd3d so far; I think things are getting to a point where it's starting to make sense to be more active about that though.
On 8/31/22 07:51, Henri Verbeet (@hverbeet) wrote:
It's mostly to avoid having to import more code than necessary. And of course for people who work on both vkd3d and Wine, using the same helpers makes life easier.
I agree that it's no big deal for list.h. I was going to look into sharing more things, like wine/debug.h or the idl headers, but I assume that's not desirable either?
It would depend a bit on the specifics, but most likely not, no. As a general principle, I think vkd3d would like to avoid cases where we'd tell people things like "Please fix this in Wine instead" or "You can't change that internal header because it would break Wine".
I know @zfigura has opinions on this subject as well, and I don't want to get too far ahead of those, but to touch a little on the subject of non-Wine users/contributors, I think it's important enough to attempt to attract those that I'd tell them "vkd3d will go out of its way to accommodate your use-cases, even if that makes things slightly harder for us as Wine developers." Going by what you wrote above, I think that's the opposite of how you see things, which is a bit unfortunate. :-\
I'm not sure I have opinions about attracting vkd3d users or developers per se, although I agree with Henri's thoughts here.
What I do find baffling is the idea that omitting the private headers is easier, or preferable, to simply importing the vkd3d source code verbatim. That is, it seems like modifying vkd3d includes is more work, both for us, and for packagers. Note also that packagers really don't like it when we make local modifications to bundled code, and I don't blame them.
I find this especially baffling when we seem to take the opposite position elsewhere in Wine; there are multiple places where we duplicate code across modules rather than introducing helper libraries (or using #include, or PARENTSRC). dmobject.c in DirectMusic in particular comes to mind, though there are other partial copies as well. What makes this case different?
It would depend a bit on the specifics, but most likely not, no. As a general principle, I think vkd3d would like to avoid cases where we'd tell people things like "Please fix this in Wine instead" or "You can't change that internal header because it would break Wine".
Noted, thanks.
This merge request was closed by Alexandre Julliard.
-
Alexandre Julliard -
Alexandre Julliard (＠julliard)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura