Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com>
--
v5: server: Don't invalidate the newly exposed child region on the parent if it doesn't clip children.
server: Redraw composited window child when its z-order changes.
server: Correctly expose composited parent window and its child on position change.
https://gitlab.winehq.org/wine/wine/-/merge_requests/231
On Wed Oct 19 11:26:44 2022 +0000, Patrick Hibbs wrote:
> Done in v3 of the patch.
> Should I respond to all of these if I think it's been done?
> Should I mark the thread resolved if I think it's been done, or is that
> something the reviewer should do?
Feel free to mark the threads that you think are done with as resolved (I think I can still reopen them if I happen to disagree)
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_12213
On Wed Oct 19 09:55:18 2022 +0000, Patrick Hibbs wrote:
> The same issue with strings occurs with Texture and shader types.
> I.e. Paramater is rendered irretrievable. Calling it's relevant GetXXX()
> function causes a crash if it's a scalar. Calling GetParamaterByName()
> will return an invalid handle.
Oh, interesting. GetParameterByName() failing afterwards is probably an indication that the SetRawValue() call overwrote parts of some internal data structure and broke the effect object to some degree. BTW, you're supposed to set texture parameters to a IDirect3DBaseTexture9 / IUnknown pointer (and similarly the value for a string parameter should probably be a string pointer rather than the string contents).
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_12212
On Wed Oct 19 09:48:02 2022 +0000, Patrick Hibbs wrote:
> Actually it seems that the strings are rendered irretrievable by the
> call to SetRawValue().
> Calling GetString() on a scalar string after the destruction causes a
> crash in win10. While calling it with a string vector doesn't crash, but
> returns D3DERR_INVALIDCALL instead.
> I've updated the tests in v3 of the patch to check for this.
Ah, that explains it I guess. Weird, but not too surprising.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_12211
On Wed Oct 19 11:23:25 2022 +0000, Patrick Hibbs wrote:
> Sorry about the unexpected push, I'll be more careful next time.
> With regards to the bool fixups, are we sure we want to just write the
> raw data given to us? I ask because I've also noticed that SetRawValue()
> has a tendency to promote datatypes when given a single value and a
> vector / matrix paramater. I.e. float and int become a float4 and int4
> respectively for both vectors and matrices. (MSDN calls this behavior a
> "cast.") This results in the next 3 values in the paramater's buffer
> being set to zero.
> Assuming SetRawValue() shouldn't fix the bool values itself, should it
> also avoid fixing the ints and floats? If so, how should we indicate the
> need for a fixup to other functions? There doesn't seem to be any
> infrastructure to support that in wine currently. (We could abuse the
> paramater block support for that, but see the speculation below.)
> The float and int promotions were the reason I didn't clear the
> paramater after each test. As doing so would cause us to miss the writes
> to the next 3 values in the paramater. As they would already be set to
> zero before the call. Should I still go ahead and clear the paramaters
> after each test anyway?
> As a dirty hack, I added a call to zero out the paramaters after each
> test with a 4x4 matrix of zero ints in v3 of the test. That call is
> commented out in the push, but if you enable it it will succeed for most
> of the tests under win10. (I disabled it before I fixed the string test
> crashes. So it may work fine now, I haven't checked.) I haven't tried
> checking the paramaters after the current one for alterations. I was
> going to put that in it's own set of tests.
> This part is speculation but, if it is a valid use of SetRawValue() to
> set adjacent paramaters regardless of type with one call, then the
> entire set of paramaters for a given effect needs to be in the same
> memory block. (Which seems to be not the case in wine, as there are
> plenty of calls to heap_alloc() / heap_realloc() using the param->data
> pointer in effect.c.) That would also imply that there is a specific
> ordering of paramaters in memory as well. Which would need to be tested
> for. If this behavior is combined with the bool assumption above, then
> we'd also need to test for order of operations too.
> Assuming all of that needs to be done, how do we split this up? Because
> I have a feeling that this patchset is going to be rather large in
> scope. (If all of the assumptions and speculation are true that is.)
Unfortunately I keep being unable to reply promptly. Anyway...
> With regards to the bool fixups, are we sure we want to just write the raw data given to us? I ask because I've also noticed that SetRawValue() has a tendency to promote datatypes when given a single value and a vector / matrix paramater. I.e. float and int become a float4 and int4 respectively for both vectors and matrices. (MSDN calls this behavior a "cast.") This results in the next 3 values in the paramater's buffer being set to zero.
Is that what happens or is just the rest of the parameter being set to 0?
> Assuming SetRawValue() shouldn't fix the bool values itself, should it also avoid fixing the ints and floats?
How should they be fixed? I'm still assuming that SetRawValue() more or less does what it says i.e. it just sets the value of parameters by mere byte-for-byte copy, without any kind of conversion or fixup. Do we have evidence that's not the case?
> The float and int promotions were the reason I didn't clear the paramater after each test. As doing so would cause us to miss the writes to the next 3 values in the paramater. As they would already be set to zero before the call. Should I still go ahead and clear the paramaters after each test anyway?
It might make sense to clear them to a non-zero value (e.g. some variation of the usual 0xdeadbeef marker) to make those results more evident.
> This part is speculation but, if it is a valid use of SetRawValue() to set adjacent paramaters regardless of type with one call, then the entire set of paramaters for a given effect needs to be in the same memory block. (Which seems to be not the case in wine, as there are plenty of calls to heap_alloc() / heap_realloc() using the param->data pointer in effect.c.) That would also imply that there is a specific ordering of paramaters in memory as well. Which would need to be tested for. If this behavior is combined with the bool assumption above, then we'd also need to test for order of operations too.
Yes, that wouldn't quite work right away. It's fine to just add a FIXME() for straddling parameters for the time being, if it turns out that native actually supports that. Similarly for the test.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/979#note_12210