Re: [PATCH 2/7] wined3d: Store vs_clipping in the adapter

[Henri Verbeet]

On 19 April 2013 11:20, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
> Potential alternatives include keeping the status quo or introducing a
> separate structure that holds these derived limits that is owned by the
> device and linked in the contexts. We could also put it into gl_info,
> but I dislike this idea because these limits are not GL limits, but
> limits of our shader and fixed function pipeline implementations.

I think we'll want a separate structure for this. Note that you
essentially have two different kinds of data you're concerned with
here though. One is derived state like e.g. the texture unit map, the
stream info, or the view_indent from patch 1/7. We don't really want
that in wined3d_state, because there's no reason for e.g. every
stateblock to have a copy of that. Having the derived state structure
be owned by the device is probably ok, although there may also be
something to be said for making it part of the context instead. The
other type of data is immutable data like derived caps and limits, as
in this patch and following ones. In principle the adapter is the
right place for that, but I do think we want a separate structure for
that, and probably a pointer to it in the context, for similar reasons
as having one to gl_info. Arguably things like the state table, format
info and the shader backend would belong in the same structure. I.e.,
essentially we'd have a setup like wined3d_gl_info, but for more
wined3d specific / derived info.

[PATCH 2/7] wined3d: Store vs_clipping in the adapter

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2013-04-19 16:26, schrieb Henri Verbeet:
> I think we'll want a separate structure for this. Note that you 
> essentially have two different kinds of data you're concerned with 
> here though. One is derived state like e.g. the texture unit map,
> the stream info, or the view_indent from patch 1/7. We don't really
> want that in wined3d_state, because there's no reason for e.g.
> every stateblock to have a copy of that. Having the derived state
> structure be owned by the device is probably ok, although there may
> also be something to be said for making it part of the context
> instead.
> My plan is to move the tex unit map and stream info into the context.
> This is fairly easy to do because it's already the context that
> triggers the update of these data structures.

The tricky bit is the lowest_disabled_stage because we have two pieces
of code that use it. One is wined3d_device_set_texture_stage_state,
which uses it to find nop state changes. The other one are the state
application function and device_map_fixed_function_samplers. The state
seems like the best place for this, as both the device and command
stream have a copy of the state where they can keep their version of
this value up to date.

IMHO it makes sense to keep view_ident close to the view matrix, as
there's a pretty direct relationship between them. The other place
where it could go is /dev/null, I doubt that it has a noticeable
performance impact. This needs some benchmarking though.

load_base_vertex_index can probably go into the context. At least I
don't see a reason that speaks against this.

There's a related situation I'm not quite happy with: The dirty states
are stored in the context, but the values that should be set are not.
For now I've kept this setup in my command stream patches and didn't
run into problems. There may be a nicer approach though.
> The other type of data is immutable data like derived caps and
> limits, as in this patch and following ones. In principle the
> adapter is the right place for that, but I do think we want a
> separate structure for that, and probably a pointer to it in the
> context, for similar reasons as having one to gl_info.
> Works for me.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRcWVLAAoJEN0/YqbEcdMwDAAP/21t5qSPPuX+CR5ur3qj9qe2
OfSJQUfmw0L5PMETxzq8TMcGI4qjcI11GxvocL9JBsft7aYxLWKJibwOcGvcdRJO
iaoHjYow2xfRMP7QgTrlP+Ii/tfpHHJHW4kw54pcKVEJnFbiDtMBGvi/IdFnXiW3
vSJAlaPojtLz2rbWSlImCiT8b3DgQ43KY4YCNSGvsG/ATXrhOpTAPPDEZ2h/28Xp
oKQ8zunob8YVWmb4zl7BJjRWJy3cj21gUgnPxLdmpqsbZn1Y6tPQj5iGOdfkb0sN
yieR1y25JNs5L6C+PrbIUW22lrrAzmvFm62f2/kugwWROdZK0gSAD2L0rdfuo5CN
xE76iZqdCoDyMy/PyEgPtMzKIwCHa7mOtfHCkpHvHHnBdLEcMTdXA7pFvKmiiZnB
bAseA+06jMl3+qphGfk4W381LCE3OUKE5t05KD1zqNhM26muh3+TC4GO702FPq+N
heIFuw3exC9f4EEyfbDJ21PtLVDFQqemWRdyfTy8UmjgFrEB9vr1HRdPnLUj4YZV
V7s/bh4/Wzco29N//1oiQ5K2CoqX+AtbnMDtPoFd5/ox8BMbJF+H4JojeX14NH+B
183dWJUC6fMve5trVenw5dkq56tv53Tls58NepH6k51TVFh7KRn5tkVDRjlfywUp
gvBE82NsnSWxDZi6rnMG
=WAaZ
-----END PGP SIGNATURE-----

[PATCH 2/7] wined3d: Store vs_clipping in the adapter

[Henri Verbeet]

On 19 April 2013 17:39, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
> The tricky bit is the lowest_disabled_stage because we have two pieces
> of code that use it. One is wined3d_device_set_texture_stage_state,
> which uses it to find nop state changes. The other one are the state
> application function and device_map_fixed_function_samplers. The state
> seems like the best place for this, as both the device and command
> stream have a copy of the state where they can keep their version of
> this value up to date.

It may be ok to just keep that in wined3d_state. I'm not sure
wined3d_device_set_texture_stage_state() would really need to touch it
though. While you could in theory avoid submitting state changes above
lowest_disabled_stage through the CS, I'm not sure it would really
gain you anything, and it could very well be more complicated than
just submitting those, and only using lowest_disabled_stage on the CS
side to avoid dirtifying states.