[PATCH 1/4] wined3d: Move the wined3d_state field of struct wined3d_cs to a new wined3d_cs_state structure.

Henri Verbeet hverbeet at gmail.com
Thu Mar 4 06:10:33 CST 2021


On Tue, 2 Mar 2021 at 22:11, Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
>
> After mulling over this and the comments on patch 4/4, I'd like to
> propose the following:
>
I think that largely makes sense. Note however that
wined3d_cs_st_require_space() is also used by
wined3d_cs_mt_require_space(), so you'll need the "data" and
associated fields in wined3d_cs_mt too. The fields for query and
present handling aren't used by wined3d_cs_st_ops, although it may
still be more convenient to keep them in the base structure. In any
case, I think that splitting wined3d_cs_mt and wined3d_cs_st is
ultimately only of secondary concern, and it would be fine to defer
that.

> In essence, this is pretty much the same thing as I was proposing with
> patch 4/4, except with different names, and with the struct inheritance
> as proposed in patch 1/4. Actually, in terms of naming, it's even closer
> to how I originally started writing this series, before I decided to try
> an approach that would thrash cs.c less.
>
> The naming is arguably still a little janky (e.g. "wined3d_cs"
> inheriting from "wined3d_device_context"), but maybe un-janky enough to
> work.
>
Well, inheritance is a (somewhat discredited) OOP concept that doesn't
exist as such in C. In case it helps, try to think about it as
composition instead of inheritance. :D

> "wined3d_device_context" is very long, though. I would also propose
> using "wined3d_queue" instead, which seems shorter and about as
> accurate. (I think I may have borrowed that one from Matteo.)
>
True, although it would by no means be the longest structure name we
have in wined3d (e.g., struct wined3d_unordered_access_view_vk). I
suppose we could use "wined3d_dc", but that's hardly unambiguous.
Historically we have tended to favour clarity over brevity for things
like (public) structure and function names, but that does mean some of
our names are a bit on the long side.

> SwapContextState() isn't valid on deferred contexts. I'm not sure
> whether we should ignore that detail in wined3d and treat it like it is,
> or effectively make it into a wined3d_device_context_ops method. Either
> way, I think we'd either need to keep state management and op queuing
> separate (as they are now), or make wined3d_device_set_state into a
> proper CS operation. I guess I don't have a strong feeling either way.
>
My initial thought would be to prefer the former, but I don't (yet)
feel strongly about it either.



More information about the wine-devel mailing list