January 2024 - Wine-GitLab - list.winehq.org

[PATCH v15 0/5] MR514: tests: Use the vulkan runner to run SM1 compilation tests.
by Francisco Casas (＠fcasas) Jan. 22, 2024

Jan. 22, 2024

Recap: My previous proposals to extend the shader_runner to SM1 were focusing on separating compilation tests ([shader] directives) from execution tests ([test] directives), and let the generic shader_runner.c:compile_shader() in charge of the former. However, Henri was more inclined to aim for making the parser agnostic to the language of the tests so we can potentially extend the shader_runner tests to other languages such as d3d-asm. This means, removing the burden of compilation from the parser altogether, and moving it to the runners, probably through a runner->compile function pointer (even if most of them end up doing the same thing, compiling with vkd3d-shader or the native compiler, depending on availability). Agreeing with going in this general direction, this MR contains patches to do SM1 compilation calling run_shader_tests() for SM1 profiles from the Vulkan runner (skipping execution for now), and some improvements to the qualifiers syntax. Despite this, there are parallel discussions on: - Whether to name the shader models individually in the [require] directives or expressing the range of shader models where the test should pass. In the latter case, whether to run for all shader models, only the lowest one in the SM1, SM4, and SM6 groups, or to allow the runner to select a shader model within the range (I feel strongly against this now, I think the runner should just retrieve a boolean in runner->check_requirements). - Where to do the iteration across different shader models, if in run_shader_tests() or expect each runner to call run_shader_tests() several times as we do now. But there is no need to settle on something for these yet. --- Now, what may be the most noisy part of this MR is that even though we are calling run_shader_tests() through the Vulkan runner, this will result in calling shader_runner.c:compile_shader() instead of shader_runner_vulkan.c:compile_shader(), but if we follow the "making the parser agnostic" plan, we eventually should get rid of shader_runner.c:compile_shader() and introduce the runner->compile pointer instead. -- v15: tests: Use the vulkan runner to run SM1 compilation tests. vkd3d-shader/ir: Lower texkill instructions to discard_nz. vkd3d-shader/d3dbc: Emit fixme for HLSL_RESOURCE_SAMPLE_LOD. tests/shader-runner: Call each runner only once. tests/shader-runner: Support reading multiple model range args for qualifiers. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/514

4 17

[PATCH v3 0/3] MR4803: dlls/gdiplus: Assorted path changes
by David Kahurani (＠kahurani) Jan. 22, 2024

Jan. 22, 2024

Signed-off-by: David Kahurani k.kahurani(a)gmail.com -- v3: dlls/gdiplus: Use path_list to path helper in GdipWidenPath. dlls/gdiplus: Use path_list to path helper in GdipFlattenPath. dlls/gdiplus: Add helper function for strictly allocating to a path https://gitlab.winehq.org/wine/wine/-/merge_requests/4803

3 5

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Vijay Kiran Kamuju (＠infyquest) Jan. 22, 2024

Jan. 22, 2024

On Mon Jan 22 18:25:33 2024 +0000, Esme Povirk wrote: > This doesn't look right to me. Assigning to desc overwrites the value > passed into the function. I will add a check if desc is already allocated or not, if yes then use realloc. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58440

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Vijay Kiran Kamuju (＠infyquest) Jan. 22, 2024

Jan. 22, 2024

On Mon Jan 22 18:34:33 2024 +0000, Esme Povirk wrote: > I think all the cases except RedEyeCorrectionEffect can be combined into > one case. I don't think the `default:` case should be reachable. They can all be combined to one, if we don't use a union and use 'void *' as member of CGpEffect to store the parameter. I am a bit confused on how to merge the cases if we use a union. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58439

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > + { > + if (paramsize != size) > + return InvalidParameter; > + } > + > + switch (effect->type) > + { > + case BlurEffect: > + alloc_copy(params, effect->p.blurparams, size); > + break; > + case SharpenEffect: > + alloc_copy(params, effect->p.sharpenparams, size); > + break; > + case TintEffect: > + alloc_copy(params, effect->p.tintparams, size); > + break; I think all the cases except RedEyeCorrectionEffect can be combined into one case. I don't think the `default:` case should be reachable. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58438

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > + > + if (effect->type == RedEyeCorrectionEffect) > + { > + if ((paramsize-size > 0) || (((size-paramsize)%sizeof(RECT)) != 0)) > + return InvalidParameter; > + } > + else > + { > + if (paramsize != size) > + return InvalidParameter; > + } > + > + switch (effect->type) > + { > + case BlurEffect: > + alloc_copy(params, effect->p.blurparams, size); I think this will leak memory if parameters were already set. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58437

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > > TRACE("(%p,%p,%u)\n", effect, params, size); > > - if(!(calls++)) > - FIXME("not implemented\n"); > + if (!effect || !params || !size) > + return InvalidParameter; > > - return NotImplemented; > + if (GdipGetEffectParameterSize(effect, &paramsize) != Ok) > + return InvalidParameter; > + > + if (effect->type == RedEyeCorrectionEffect) > + { > + if ((paramsize-size > 0) || (((size-paramsize)%sizeof(RECT)) != 0)) > + return InvalidParameter; I don't think we can use `paramsize` in this case because it will depend on whether parameters were already set. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58436

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > status = InvalidParameter; > break; > } > - > *size = sz; > + > return status; > } > > +void alloc_copy(const void *src, void *desc, UINT size) > +{ > + desc = malloc(size); > + memcpy(desc, src, size); > +} This doesn't look right to me. Assigning to desc overwrites the value passed into the function. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58435

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/image.c: > stat = pGdipCreateEffect(BlurEffectGuid, NULL); > expect(InvalidParameter, stat); > > + stat = pGdipGetEffectParameterSize(NULL, NULL); > + expect(InvalidParameter, stat); > + > + stat = pGdipGetEffectParameterSize(effect, NULL); This uses effect uninitialized. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58434

1 0

[PATCH v4 0/2] MR4830: dinput: implement DIPROP_AUTOCENTER
by Tyson Whitehead (＠twhitehead) Jan. 22, 2024

Jan. 22, 2024

Autocenter is a default effect playing on the device following power up or reset. It is disabled by stopping all effects. On at least some devices (all?) it is a spring effect playing in slot 1. Capturing Windows USB packets reveals it dinput acquire does (1) a reset (this enabled autocenter) and, if autocenter is disabled, (2) a stop all effects (this disabled autocenter). This logic works regardless of whether autocenter is a spring effect playing in slot 1 or not. It does mean autocenter can only be set when the device is not acquired. Testing on Windows reveals setting autocenter properties while acquired returns DIERR_ACQUIRED even if the device is exclusively acquired, so this is consistent. -- v4: dinput: Update tests for DIPROP_AUTOCENTER dinput: Implement DIPROP_AUTOCENTER https://gitlab.winehq.org/wine/wine/-/merge_requests/4830

4 11