June 2022 - Wine-Devel - list.winehq.org

Re: [PATCH 1/5] winegstreamer: Don't try using src_getrange_cb input buffer.

by Zebediah Figura

On 6/14/22 02:26, Rémi Bernon wrote: > From: Rémi Bernon <rbernon(a)codeweavers.com> > > It is invalid according to GStreamer documentation. What documentation are you referring to? The documentation for GstPadGetRangeFunction itself is somewhat badly written and doesn't actually mention what "buffer" is expected to contain on input, but the documentation for gst_pad_get_range() says: If buffer points to a variable holding NULL, a valid new GstBuffer will be placed in buffer when this function returns GST_FLOW_OK. The new buffer must be freed with gst_buffer_unref after usage. When buffer points to a variable that points to a valid GstBuffer, the buffer will be filled with the result data when this function returns GST_FLOW_OK. If the provided buffer is larger than size, only size bytes will be filled in the result buffer and its size will be updated accordingly. and the implementation shows that the intent is for the same to apply to GstPadGetRangeFunction. For what it's worth, I believe oggdemux actually does pass a non-NULL buffer, and breaks if we don't fill that.

June 16, 2022

Re: [PATCH v2 0/5] MR131: wined3d: Introduce wined3d_device_check_format_support().

by Zebediah Figura

On 6/15/22 18:38, Chip Davis (@cdavis5e) wrote: > Since you've started on Henri's approach, I guess there's no need for this MR anymore. > I hadn't actually started converting anything (nor did I immediately intend to); I was just trying to fix a couple of bugs with the current implementation.

June 15, 2022

Re: [PATCH 0/4] MR233: win32u: Partially move raw input APIs.

by Zebediah Figura

On 6/14/22 00:02, Rémi Bernon (@rbernon) wrote: > On Tue Jun 14 04:05:11 2022 +0000, **** wrote: >> Zebediah Figura replied on the mailing list: >> ``` >> On 6/13/22 03:27, Rémi Bernon (@rbernon) wrote: >>> Rémi Bernon (@rbernon) commented about dlls/user32/tests/input.c: >>>> + ok(count == ~0u, "GetRawInputData returned %d\n", count); >>>> + ok(GetLastError() == ERROR_INVALID_PARAMETER, >> "GetRawInputData returned %08lx\n", GetLastError()); >>>> + } >>>> + else if (is_wow64) >>>> + { >>>> + count = GetRawInputData((HRAWINPUT)lparam, >> RID_INPUT, &ri, &size, sizeof(RAWINPUTHEADER64)); >>>> + todo_wine ok(count == sizeof(ri), "GetRawInputData >> returned %d\n", count); >>>> + ok(ri.data.mouse.lLastX == 6, "Unexpected rawinput >> data: %ld\n", ri.data.mouse.lLastX); >>>> + todo_wine ok(GetLastError() == 0xdeadbeef, >> "GetRawInputData returned %08lx\n", GetLastError()); >>>> + } >>>> + else >>>> + { >>>> + count = GetRawInputData((HRAWINPUT)lparam, >> RID_INPUT, &ri, &size, sizeof(RAWINPUTHEADER64)); >>>> + ok(count == ~0u, "GetRawInputData returned %d\n", count); >>>> + ok(GetLastError() == ERROR_INVALID_PARAMETER, >> "GetRawInputData returned %08lx\n", GetLastError()); >>>> + } >>> It feels a bit unbalanced to test invalid cases at the same time as a >> valid case depending on the arch. Could we have instead a test for the >> invalid sizes, and the another one with valid size? >> I don't understand what you mean by this. Only one of these three tests >> makes sense, and on mutually exclusive architectures. >> I could separate the if/elif/else into three separate if blocks, but I >> doubt that's what you mean. >> ``` > I mean that, like the `GetRawInputBuffer` tests below you check an invalid size and ERROR_INVALID_PARAMETER error case, but on wow64, you don't and instead you check a valid size and the returned data. > > I think you should check the same cases on all arch if possible, so invalid size and error on all arch (similarly to `GetRawInputBuffer`), then the valid case and data on all arch as well. > The point is to check the "wrong" architecture's size in all three cases. In the wow64 case, this succeeds; otherwise, it fails. This quirk is specific to GetRawInputData(); GetRawInputBuffer() always rejects the "wrong" architecture's size. Perhaps this structure would make it clearer: if (sizeof(void *) == 8) { count = GetRawInputData(..., sizeof(RAWINPUTHEADER32)); ok(count == ~0u, ...); } else { count = GetRawInputData(..., sizeof(RAWINPUTHEADER64)); if (is_wow64) { ok(count == sizeof(ri), ...); } else { ok(count == ~0u, ...); } }

June 15, 2022

Re: [PATCH v2 0/5] MR131: wined3d: Introduce wined3d_device_check_format_support(). - closed

by Chip Davis (＠cdavis5e)

This merge request was closed by Chip Davis. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/131

June 15, 2022

Re: [PATCH v2 0/5] MR131: wined3d: Introduce wined3d_device_check_format_support().

by Chip Davis (＠cdavis5e)

Since you've started on Henri's approach, I guess there's no need for this MR anymore. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/131#note_2165

June 15, 2022

Re: [PATCH 0/5] MR237: winegstreamer: Implement Resampler and ColorConvert MF transforms. - approved

by Zebediah Figura (＠zfigura)

This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/237

June 15, 2022

Re: HLSL offsetting

by Zebediah Figura

On 6/14/22 09:49, Giovanni Mascellani wrote: > Hi, > > I finally managed to read through your messages, and it seems that we > have a sort of agreement for trying with structured dereferences, which > I agree with too. > > Il 10/06/22 23:13, Zebediah Figura ha scritto: >>> On the other hand, the version of (b) I imagine changes hlsl_deref to: >>> --- >>> struct hlsl_deref >>> { >>> struct hlsl_ir_var *var; >>> unsigned int route_len; >>> struct hlsl_src *route; >>> }; >>> --- >>> Where route is intended to be a variable size array of component >>> offsets. >> >> I was envisioning something more explicit, but this is simpler, so my >> guess is that this is what we want. > > I can live with Francisco's proposal, but my preference would be for > something like: > > enum hlsl_selector_type > { > HLSL_SELECTOR_SUBSCRIPT, > HLSL_SELECTOR_FIELD, > }; > > struct hlsl_selector > { > enum hlsl_selector_type type; > union > { > hlsl_src *subscript; > unsigned int field; > }; > }; > > struct hlsl_deref > { > struct hlsl_ir_var *var; > unsigned int path_len; > struct hlsl_selector *path; > }; > > This way field indices are kept as actual numbers, not other nodes that > have to resolve to a constant uint value. In general I like data types > to constrain as much as possible to the valid values that you can store > in them (and I don't think there is any way in HLSL to make a dynamic > reference to a field of a structure, like it is possible in C++). I agree in principle, but this does make the structure more complex to deal with. I'm inclined to stick to Francisco's proposal for now and just store it as a constant either way. We could certainly revisit it later, though.

June 15, 2022

Re: HLSL offsetting

by Zebediah Figura

On 6/14/22 22:05, Francisco Casas wrote: > Hello, > > On 13-06-22 11:59, Francisco Casas wrote: >> Okay, I stopped working on multiple register offsets and now I am >> working on this approach. >> >> I still think that adding a pass to translate these "route"s to the >> original register "offset"s, so that we can implement this change >> gradually, is good. We can move the pass forward as we change more >> passes to work with component offsets. And we can debug more easily >> the things that we don't translate correctly. >> >> I am aiming to implement the new approach until right after >> split_matrix_copies, and put the translation afterwards. So far it >> seems to be going nicely. >> > > I attach a patch that achieves this, in case there are any opinions. > > The "paths"[1] of component indexes seem like the best way to solve the > problem after all. I think that keeping them as an arrays of nodes in > the hlsl_deref allows to write more uniform code than using a more > complex data structure, as Giovanni mentioned. > > IMO, I think this patch manages matrices nicely too, but there is the > problem that it is based on top of > --- > bb49bdba gmascellani vkd3d-shader/hlsl: Allow majority modifiers on > function declarations. > --- > which is a little behind of master. > > So I still have to rebase it on top of current master, and in particular > on top of the recent patches pertaining matrices. So I will probably > have to make some design decisions while solving the conflicts. > > I guess that the main objective is that we don't want to deal with > matrices at all after splitting matrix copies. From skimming the patch, here are some immediate thoughts: * I have a patch which has been around for a while which replaces the field linked list with an array. I think we wanted that anyway, but we especially want it given list_get(). * All current (upstream) uses of hlsl_new_store() and hlsl_new_load() with a nonzero offset fall into one of two categories: - retrieving the element at a specific offset (initializer and matrix parsing) - adding another offset to an existing hlsl_deref (splitting passes, initial parsing of field/element derefs) I might have missed something, but assuming I've got that right, I think that the hlsl_new_load() and hlsl_new_store() should take arguments to reflect that. (I.e. split each into two functions, one taking an unsigned int and the other taking a const struct hlsl_deref *). In that case struct path_arg would be limited to hlsl.c, if it even needs to exist. Note that this step could also be done independently of anything else, which would help make the "big" patch more reviewable. * Similarly I think hlsl_new_resource_load() and friends should just take a "const struct hlsl_deref *" or two. I'm not immediately sure what to do with prepend_input_copy() and friends—maybe those should be converted to iterative passes instead of recursive ones. * Stuff like free_hlsl_deref() can be made into a separate patch.

June 15, 2022

Re: [PATCH v6 0/1] MR167: d3drm: IDirect3DRMMeshBuilder3::Load support loading from resource

by Zebediah Figura

I've pushed a new version which fixes up a few stylistic nitpicks.

June 15, 2022

Re: [PATCH v6 0/1] MR167: d3drm: IDirect3DRMMeshBuilder3::Load support loading from resource - approved

by Zebediah Figura (＠zfigura)

This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/167

June 15, 2022