- Wine-gitlab - list.winehq.org

[PATCH 0/5] MR1796: Fix some winemac.drv compiler warnings
by Huw Davies (＠huw) 12 Dec '22

12 Dec '22

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1796

2 5

Re: [PATCH v2 0/6] MR1682: d3dx9: Partially implement texture filling using texture shaders.
by Matteo Bruni (＠Mystral) 12 Dec '22

12 Dec '22

Sorry I didn't get to your MRs earlier, I was rushing to try and finish cleaning up the d3dx9 tests for long integer types, which I also couldn't manage to do before the deadline :/ I left a few comments, nothing too crazy. Probably going to defer the MRs until the code freeze is lifted? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1682#note_19501

1 0

Re: [PATCH v2 0/6] MR1682: d3dx9: Partially implement texture filling using texture shaders.
by Matteo Bruni (＠Mystral) 12 Dec '22

12 Dec '22

Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/texture.c: > + hr = IDirect3DCubeTexture9_GetLevelDesc(texture, m, &desc); > + ok(hr == D3D_OK, "Unexpected hr %#x.\n", hr); > + > + size = desc.Width; > + > for (f = 0; f < 6; f++) > { > - hr = IDirect3DCubeTexture9_LockRect(tex, f, 0, &lock_rect, NULL, D3DLOCK_READONLY); > + texelsize.x = (f == 0) || (f == 1) ? 0.0f : 2.0f / size; > + texelsize.y = (f == 2) || (f == 3) ? 0.0f : 2.0f / size; > + texelsize.z = (f == 4) || (f == 5) ? 0.0f : 2.0f / size; > + > + hr = IDirect3DCubeTexture9_LockRect(texture, f, m, &lock_rect, NULL, D3DLOCK_READONLY); > ok(hr == D3D_OK, "Couldn't lock the texture, error %#x\n", hr); > if (SUCCEEDED(hr)) > { We should just get rid of the if, here or in a separate (preceding) patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1682#note_19500

1 0

Re: [PATCH v2 0/6] MR1682: d3dx9: Partially implement texture filling using texture shaders.
by Matteo Bruni (＠Mystral) 12 Dec '22

12 Dec '22

Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/texture.c: > + {XCOORD, ZERO, YCOORDINV}, > + {XCOORD, YCOORDINV, ONE}, > + {XCOORDINV, YCOORDINV, ZERO} > + }; > > - if (SUCCEEDED(hr)) > + DWORD x, y, m, f, levels, size, value, expected; > + D3DXVECTOR3 coord, texelsize; > + D3DLOCKED_RECT lock_rect; > + D3DSURFACE_DESC desc; > + D3DXVECTOR4 out; > + HRESULT hr; > + > + levels = IDirect3DCubeTexture9_GetLevelCount(texture); > + > + for (m = 0; m < levels; m++) Given that you're touching these lines anyway, you can probably replace the postincrement operator with preincrement (we use the latter by default). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1682#note_19499

1 0

Re: [PATCH v2 0/6] MR1682: d3dx9: Partially implement texture filling using texture shaders.
by Matteo Bruni (＠Mystral) 12 Dec '22

12 Dec '22

Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/texture.c: > + return 0; > + } > +} > > - IDirect3DCubeTexture9_Release(tex); > +static DWORD get_expected_argb_color(D3DFORMAT format, const D3DXVECTOR4 *v) > +{ > + switch (format) > + { > + case D3DFMT_A8R8G8B8: > + return (BYTE)(v->w * 255 + 0.5f) << 24 > + | (BYTE)(v->x * 255 + 0.5f) << 16 > + | (BYTE)(v->y * 255 + 0.5f) << 8 > + | (BYTE)(v->z * 255 + 0.5f); > + case D3DFMT_A1R5G5B5: > + Probably the empty line was meant to go above the case rather than below. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1682#note_19498

1 0

Re: [PATCH v2 0/6] MR1682: d3dx9: Partially implement texture filling using texture shaders.
by Matteo Bruni (＠Mystral) 12 Dec '22

12 Dec '22

Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/texture.c: > - { > - hr = IDirect3DCubeTexture9_LockRect(tex, f, m, &lock_rect, NULL, D3DLOCK_READONLY); > - ok(hr == D3D_OK, "Couldn't lock the texture, error %#x\n", hr); > - if (SUCCEEDED(hr)) > - { > - pitch = lock_rect.Pitch / sizeof(DWORD); > - for (y = 0; y < size; y++) > - { > - for (x = 0; x < size; x++) > - { > - value = ((DWORD *)lock_rect.pBits)[y * pitch + x]; > - v[0] = (value >> 24) & 0xff; > - v[1] = (value >> 16) & 0xff; > - v[2] = (value >> 8) & 0xff; > - v[3] = value & 0xff; > + ret = (value >> 15 & 0x1f) << 16 Shouldn't that be ```suggestion:-0+0 ret = value >> 15 << 24 ``` ? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1682#note_19497

1 0

Re: [PATCH v2 0/6] MR1682: d3dx9: Partially implement texture filling using texture shaders.
by Matteo Bruni (＠Mystral) 12 Dec '22

12 Dec '22

Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/shader.c: > + *(inputs++) = 0.0f; > + *(inputs++) = 0.0f; > + > + param.type = D3DXPT_FLOAT; > + param.bytes = 4 * sizeof(float); > + d3dx_evaluate_parameter(shader->eval, &param, out); > +} > + > +HRESULT WINAPI D3DXFillTextureTX(struct IDirect3DTexture9 *texture, ID3DXTextureShader *texture_shader) > +{ > + struct d3dx9_texture_shader *shader = unsafe_impl_from_ID3DXTextureShader(texture_shader); > + > + TRACE("texture %p, texture_shader %p.\n", texture, texture_shader); > + > + return D3DXFillTexture(texture, texture_shader_fill_2d, shader); > +} This doesn't look too great at the first sight but, thinking about it, I don't really see any clearly nicer alternative. Which is to say, this is probably fine :D -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1682#note_19495

1 0

Re: [PATCH v2 0/6] MR1682: d3dx9: Partially implement texture filling using texture shaders.
by Matteo Bruni (＠Mystral) 12 Dec '22

12 Dec '22

Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/effect.c: > - if (effect->params.count) > + if (FAILED(hr = d3dx_init_parameters_store(&effect->params, params_count))) > { > - effect->params.parameters = heap_alloc_zero(sizeof(*effect->params.parameters) * effect->params.count); > - if (!effect->params.parameters) > - { > - ERR("Out of memory.\n"); > - hr = E_OUTOFMEMORY; > - goto err_out; > - } > + hr = E_OUTOFMEMORY; > + goto err_out; > + } > > + if (effect->params.count) > + { You can just get rid of the if entirely at this point. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1682#note_19494

1 0

Re: [PATCH v2 0/6] MR1682: d3dx9: Partially implement texture filling using texture shaders.
by Matteo Bruni (＠Mystral) 12 Dec '22

12 Dec '22

Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/preshader.c: > struct d3dx_parameters_store *parameters) > { > unsigned int *p; > - unsigned int i, j, const_count; > + unsigned int i, j, const_count, magic; > double *dconst; > HRESULT hr; > unsigned int saved_word; > unsigned int section_size; > > + magic = *ptr; > + > TRACE("Preshader version %#x.\n", *ptr & 0xffff); It seems nice to also update the TRACE() to output the full 32-bit value. Additional nitpick: you can probably drop the "for 2 registers" part of the patch subject. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1682#note_19493

1 0

[PATCH v3 0/1] MR1766: d3dx9_36/tests: Get rid of GCC12 warning.
by eric pouech (＠epo) 12 Dec '22

12 Dec '22

Follow GCC recommendations for getting rid of it. MingW/GCC 12 complains with: /home/eric/work/wine/dlls/d3dx9_36/tests/effect.c: In function 'test_effect_parameter_value': /home/eric/work/wine/dlls/d3dx9_36/tests/effect.c:1838:71: warning: expression does not compute the number of elements in this array; element type is 'DWORD' {aka 'long unsigned int'}, not 'D3DXMATRIX' {aka 'struct _D3DMATRIX'} [-Wsizeof-array-div] 1838 | const D3DXMATRIX *matrix_pointer_array[sizeof(input_value)/sizeof(D3DXMATRIX)]; | ^ /home/eric/work/wine/dlls/d3dx9_36/tests/effect.c:1838:71: note: add parentheses around the second 'sizeof' to silence this warning /home/eric/work/wine/dlls/d3dx9_36/tests/effect.c:1835:19: note: array 'input_value' declared here 1835 | DWORD input_value[EFFECT_PARAMETER_VALUE_ARRAY_SIZE]; | ^~~~~~~~~~~ Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> -- v3: d3dx9/tests: Avoid a GCC warning (-Wsizeof-array-div). https://gitlab.winehq.org/wine/wine/-/merge_requests/1766

3 4