June 2016 - Wine-devel - list.winehq.org

Re: [PATCH 4/6] wined3d: Create texture views for shader resource views.
by Henri Verbeet 06 Jun '16

06 Jun '16

On 6 June 2016 at 11:39, Józef Kucia <jkucia(a)codeweavers.com> wrote: > +static BOOL gl_internal_format_equal(const struct wined3d_format *format1, > + const struct wined3d_format *format2) > +{ > + return format1->id == format2->id > + || (format1->glInternal == format2->glInternal > + && format1->glGammaInternal == format2->glGammaInternal > + && format1->rtInternal == format2->rtInternal); > +} When would you use this? I was under the impression that Direct3D views were a bit more restricted then OpenGL views and only work between formats based on the same typeless format. > +static void wined3d_shader_resource_view_create_texture_view(struct wined3d_shader_resource_view *view, > + const struct wined3d_shader_resource_view_desc *desc, const struct wined3d_format *view_format, > + struct wined3d_texture *texture) > +{ > + struct wined3d_context *context = context_acquire(texture->resource.device, NULL); > + const struct wined3d_gl_info *gl_info = context->gl_info; > + struct gl_texture *gl_texture; This is fairly minor, but context_acquire() potentially does real work. It seems somewhat inappropriate to "hide" it in the declarations.

2 1

Re: [03/10] msvcirt: Implement ostream::flush
by Piotr Caban 06 Jun '16

06 Jun '16

On 06/06/16 13:52, Iván Matellanes wrote: > DEFINE_THISCALL_WRAPPER(ostream_flush, 4) > ostream* __thiscall ostream_flush(ostream *this) > { > - FIXME("(%p) stub\n", this); > + ios *base = ostream_get_ios(this); > + > + TRACE("(%p)\n", this); > + > + if (call_streambuf_sync(base->sb) == EOF) > + ios_clear(base, base->state | IOSTATE_failbit); This function should be thread-safe. Please use ios_lockbuf and ios_unlockbuf. Thanks, Piotr

1 0

Re: [PATCH 1/6] wined3d: Add wined3d_shader_resource_view_desc.
by Henri Verbeet 06 Jun '16

06 Jun '16

On 6 June 2016 at 11:39, Józef Kucia <jkucia(a)codeweavers.com> wrote: > +enum wined3d_view_type > +{ > + WINED3D_VTYPE_BUFFER = 1, > + WINED3D_VTYPE_TEXTURE_1D = 2, > + WINED3D_VTYPE_TEXTURE_1D_ARRAY = 3, > + WINED3D_VTYPE_TEXTURE_2D = 4, > + WINED3D_VTYPE_TEXTURE_2D_ARRAY = 5, > + WINED3D_VTYPE_TEXTURE_3D = 6, > + WINED3D_VTYPE_TEXTURE_CUBE = 7, > + WINED3D_VTYPE_TEXTURE_CUBE_ARRAY = 8, > +}; > + Not a fan of the "VTYPE" in the enum elements. It does match "RTYPE" from enum wined3d_resource_type, but that is itself a leftover from when it more closely matched D3DRESOURCETYPE. > +struct wined3d_shader_resource_view_desc > +{ > + enum wined3d_format_id format_id; > + enum wined3d_view_type view_type; > + union > + { > + struct > + { > + unsigned int start_idx; > + unsigned int count; > + unsigned int flags; > + } buffer; > + struct > + { > + unsigned int level_idx; > + unsigned int level_count; > + unsigned int layer_idx; > + unsigned int layer_count; > + } texture; > + } u; > +}; Is the "view_type" field needed? I seem to remember that the view type needs to match the resource type, so you shouldn't be able to e.g. create a texture view on a buffer resource. The main other reasons I can imagine for having the field are cube views, and perhaps shaders making a distinction between array textures with a single layer and non-array textures. The latter could easily be handled by setting "layer_count" to ~0u for non-array textures, but since we have a "flags" field with plenty of extra bits, I think we could just use that to mark both cube and array views.

2 1

Re: [01/10] msvcirt: Add stub for class ostream
by Piotr Caban 06 Jun '16

06 Jun '16

On 06/06/16 13:52, Iván Matellanes wrote: > +DEFINE_THISCALL_WRAPPER(ostream_vector_dtor, 8) > +ostream* __thiscall ostream_vector_dtor(ostream *this, unsigned int flags) ostream_vector_dtor should probably take ios* as first argument. > +/* ??_Gostream@@UAEPAXI(a)Z */ > +DEFINE_THISCALL_WRAPPER(ostream_scalar_dtor, 8) > +ostream* __thiscall ostream_scalar_dtor(ostream *this, unsigned int flags) The same here. Thanks, Piotr

1 0

Re: [02/10] msvcirt: Implement ostream constructors/destructors
by Piotr Caban 06 Jun '16

06 Jun '16

On 06/06/16 13:52, Iván Matellanes wrote: > DEFINE_THISCALL_WRAPPER(ostream_copy_ctor, 12) > ostream* __thiscall ostream_copy_ctor(ostream *this, const ostream *copy, BOOL virt_init) > { > - FIXME("(%p %p %d) stub\n", this, copy, virt_init); > + ios *base, *copy_base; > + > + TRACE("(%p %p %d)\n", this, copy, virt_init); > + > + if (virt_init) > + this->vbtable = ostream_vbtable; > + base = ostream_get_ios(this); > + copy_base = ostream_get_ios(copy); > + ios_sb_ctor(base, copy_base->sb); > + base->vtable = &MSVCP_ostream_vtable; This is wrong if virt_init == FALSE. There should be something like: copy_base = ostream_get_ios(copy); if (virt_init) { this->vbtable = ostream_vbtable; base = ostream_get_ios(thjs); ios_sb_ctor(base, copy_base->sb); } else { base = ostream_get_ios(this); ios_init(base, copy_base->sb); } Keep in mind that this code was not tested. Please also add a test for ostream_copy_ctor(..., FALSE) call. Thanks, Piotr

1 0

Re: [PATCH 2/2] wined3d: Add some more Intel GPUs.
by Henri Verbeet 06 Jun '16

06 Jun '16

On 4 June 2016 at 22:07, Grazvydas Ignotas <notasas(a)gmail.com> wrote: > For HD Graphics 520 or newer, the whole system RAM seems to be reported > as VRAM. For now I've chosen 2048MB based on assumption that systems as > new as this will have at least 2GB of RAM. > > Signed-off-by: Grazvydas Ignotas <notasas(a)gmail.com> > --- > dlls/wined3d/directx.c | 5 +++++ > dlls/wined3d/wined3d_private.h | 5 +++++ > 2 files changed, 10 insertions(+) > This is fine, but could you also please add a generic "Skylake" fallback to cards_intel[] for cards with a different device ID?

1 0

Re: [PATCH 2/6] d3d11: Normalize more fields in SRV descriptions.
by Henri Verbeet 06 Jun '16

06 Jun '16

On 6 June 2016 at 11:39, Józef Kucia <jkucia(a)codeweavers.com> wrote: > + case D3D11_SRV_DIMENSION_TEXTURE2DARRAY: > + if (desc->u.Texture2DArray.MipLevels == -1 && desc->u.Texture2DArray.MostDetailedMip < miplevel_count) > + desc->u.Texture2DArray.MipLevels = miplevel_count - desc->u.Texture2DArray.MostDetailedMip; > + if (desc->u.Texture2DArray.ArraySize == -1 && desc->u.Texture2DArray.FirstArraySlice < layer_count) > + desc->u.Texture2DArray.ArraySize = layer_count - desc->u.Texture2DArray.FirstArraySlice; > + break; Does that work for render target views as well?

1 0

Re: [PATCH 2/3] d3drm/tests: Add tests for IDirect3DRM*::CreateViewport.
by Stefan Dösinger 06 Jun '16

06 Jun '16

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Am 2016-06-05 um 19:28 schrieb Aaryaman Vasishta: > + hr = IDirect3DRM_CreateViewport(d3drm1, device1, frame, 0, 0, 0, 0, &viewport); > + ok(SUCCEEDED(hr), "Cannot get IDirect3DRMViewport interface (hr = %x)\n", hr); > + ref4 = get_refcount((IUnknown *)d3drm1); > + todo_wine ok(ref4 > ref1, "Expected ref4 > ref1, got ref1 = %u, ref4 = %u.\n", ref1, ref4); > + ref4 = get_refcount((IUnknown *)d3drm2); > + ok(ref4 == ref2, "Expected ref4 == ref2, got ref2 = %u, ref4 = %u.\n", ref2, ref4); > + ref4 = get_refcount((IUnknown *)d3drm3); > + ok(ref4 == ref3, "Expected ref4 == ref3, got ref3 = %u, ref4 = %u.\n", ref3, ref4); Does creating the viewport have any influence of the device's or the frame's refcount? > + hr = IDirect3DRM_CreateViewport(d3drm1, device1, frame, -1, 0, 0, 0, &viewport); > + todo_wine ok(hr == D3DRMERR_BADVALUE, "Expected hr == D3DRMERR_BADVALUE, got %#x.\n", hr); x, y, width and height are DWORDs. -1 only makes limited sense here. It is probably better to test width and height = 0 and test x and (x + width) against the device's render target size. (smaller, matching, larger. Also obviously the same with y + height). -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXVKOEAAoJEN0/YqbEcdMwKvsP/jzDXf0eCqtCjXtfEiOlknWL iaBRAFHJnFz7fhFX1yZDX63HScoksoWw3TAby6LDX8zA0oVMWP7IYLXk+lx4xPTg mj8+lhvyPNPDl+PQ4feU/1sAwByBVR9qn6kwxgF6nsW0RXqalCeztcMxLOkC5kXZ bGbt/t4ZaCNVyZLjv5tYOcUHq+MP9Q8cKnbq3cGaivsd/cab4ORfpFccstBqLlL+ LJD65/oCtq9A+OGORVxuin7CFszSP9OR3/vFiCDMvJFjIFMWSss4gTxF4vvamD9+ Fie3E85DBWr3fBomKlKEPeky7LW0QBv5NcdzJhLaXiaCkxgKd5s54v70+2C4OSEE /N1P5v4vCjfJ8ZhFKyXJmTa6NPO0qXTnGY+HREmhva/aemTG706DaS58mWacovr1 +xx5uvMcuG2dQUGqDjp3KVKz+wLnPovfn3WoEtQ82Tvgq3g2aJbsEKHhFUjkscpG l/Ck0NcZMnQA7OxREO8+D2R7rSI+WdLyKHTrPmWc8HJ/5EWL2LE2YkdwNvjzpbTI 7fWHd5PE1NFdw7xzcHajq62485fZe1T+vt1qkMO70qjRkvlGgW/OI1VzVr7GyhSp UBxXTHrb32rQn3UD36SAYiGh6pF5HAszSAUVeesUx0VEAsP/CazZS0HHOP5fksrc MmZ1+t0cL04S8q0bGxLc =9s0i -----END PGP SIGNATURE-----

2 1

Re: user32/tests: Use EqualRect() instead of open coding it.
by Marvin 06 Jun '16

06 Jun '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=23360 Your paranoid android. === w7u (32 bit msg) === msg.c:4773: Test failed: ShowWindow(SW_RESTORE):overlapped: 18: the msg sequence is not complete: expected 0000 - actual 0088 === w864 (32 bit msg) === msg.c:6484: Test failed: Update region shouldn't be empty msg.c:6717: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6719: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6490: Test failed: Regions are different msg.c:6490: Test failed: Regions are different msg.c:6490: Test failed: Regions are different === w1064 (32 bit msg) === msg.c:6484: Test failed: Update region shouldn't be empty msg.c:6717: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6719: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6490: Test failed: Regions are different msg.c:6490: Test failed: Regions are different msg.c:6490: Test failed: Regions are different === w864 (64 bit msg) === msg.c:6484: Test failed: Update region shouldn't be empty msg.c:6717: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6719: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6490: Test failed: Regions are different msg.c:6490: Test failed: Regions are different msg.c:6490: Test failed: Regions are different === w1064 (64 bit msg) === msg.c:6484: Test failed: Update region shouldn't be empty msg.c:6717: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6719: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6490: Test failed: Regions are different msg.c:6490: Test failed: Regions are different msg.c:6490: Test failed: Regions are different

2 1

Re: [PATCH] wined3d: Add #defines and logs for missing new SM5 modifiers.
by Józef Kucia 06 Jun '16

06 Jun '16

This patch isn't very useful on its own. It just changes FIXME() messages. It would be much better to save the decoded modifiers in wined3d_shader_instruction and print them in the shader tracing backend (see shader_trace_init() in shader.c). Other than that, the patch has some coding style issues. On Thu, Jun 2, 2016 at 3:49 PM, Guillaume Charifi <guillaume.charifi(a)sfr.fr> wrote: > Signed-off-by: Guillaume Charifi <guillaume.charifi(a)sfr.fr> > --- > dlls/wined3d/shader_sm4.c | 78 ++++++++++++++++++++++++++++++++--------------- > 1 file changed, 54 insertions(+), 24 deletions(-) > > diff --git a/dlls/wined3d/shader_sm4.c b/dlls/wined3d/shader_sm4.c > index eb8e4e5..3adaf70 100644 > --- a/dlls/wined3d/shader_sm4.c > +++ b/dlls/wined3d/shader_sm4.c > @@ -25,6 +25,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(d3d_shader); > WINE_DECLARE_DEBUG_CHANNEL(d3d_bytecode); > > #define WINED3D_SM4_INSTRUCTION_MODIFIER (0x1u << 31) > +#define WINED3D_SM4_MODIFIER_TYPE_MASK 0x3f > > #define WINED3D_SM4_MODIFIER_AOFFIMMI 0x1 > #define WINED3D_SM4_AOFFIMMI_U_SHIFT 9 > @@ -34,6 +35,22 @@ WINE_DECLARE_DEBUG_CHANNEL(d3d_bytecode); > #define WINED3D_SM4_AOFFIMMI_W_SHIFT 17 > #define WINED3D_SM4_AOFFIMMI_W_MASK (0xfu << WINED3D_SM4_AOFFIMMI_W_SHIFT) > > +#define WINED3D_SM5_MODIFIER_RESDIM 0x2 > +#define WINED3D_SM5_RESDIM_RES_TYPE_SHIFT 6 > +#define WINED3D_SM5_RESDIM_RES_TYPE_MASK (0x1fu << WINED3D_SM5_RESDIM_RES_TYPE_SHIFT) > +#define WINED3D_SM5_RESDIM_BUF_STRIDE_SHIFT 11 > +#define WINED3D_SM5_RESDIM_BUF_STRIDE_MASK (0xfffu << WINED3D_SM5_RESDIM_BUF_STRIDE_SHIFT) > + > +#define WINED3D_SM5_MODIFIER_RETTYPE 0x3 > +#define WINED3D_SM5_RETTYPE_X_SHIFT 6 > +#define WINED3D_SM5_RETTYPE_X_MASK (0xfu << WINED3D_SM5_RETTYPE_X_SHIFT) > +#define WINED3D_SM5_RETTYPE_Y_SHIFT 10 > +#define WINED3D_SM5_RETTYPE_Y_MASK (0xfu << WINED3D_SM5_RETTYPE_Y_SHIFT) > +#define WINED3D_SM5_RETTYPE_Z_SHIFT 14 > +#define WINED3D_SM5_RETTYPE_Z_MASK (0xfu << WINED3D_SM5_RETTYPE_Z_SHIFT) > +#define WINED3D_SM5_RETTYPE_W_SHIFT 18 > +#define WINED3D_SM5_RETTYPE_W_MASK (0xfu << WINED3D_SM5_RETTYPE_W_SHIFT) > + > #define WINED3D_SM4_INSTRUCTION_LENGTH_SHIFT 24 > #define WINED3D_SM4_INSTRUCTION_LENGTH_MASK (0x1fu << WINED3D_SM4_INSTRUCTION_LENGTH_SHIFT) > > @@ -1163,32 +1180,45 @@ static BOOL shader_sm4_read_dst_param(struct wined3d_sm4_data *priv, const DWORD > > static void shader_sm4_read_instruction_modifier(DWORD modifier, struct wined3d_shader_instruction *ins) > { > - static const DWORD recognized_bits = WINED3D_SM4_INSTRUCTION_MODIFIER > - | WINED3D_SM4_MODIFIER_AOFFIMMI > - | WINED3D_SM4_AOFFIMMI_U_MASK > - | WINED3D_SM4_AOFFIMMI_V_MASK > - | WINED3D_SM4_AOFFIMMI_W_MASK; > - > - if (modifier & ~recognized_bits) > + switch (modifier & WINED3D_SM4_MODIFIER_TYPE_MASK) > { > - FIXME("Unhandled modifier 0x%08x.\n", modifier); > - } > - else > - { > - /* Bit fields are used for sign extension */ > - struct > + case WINED3D_SM4_MODIFIER_AOFFIMMI: > { > - int u : 4; > - int v : 4; > - int w : 4; > - } > - aoffimmi; > - aoffimmi.u = (modifier & WINED3D_SM4_AOFFIMMI_U_MASK) >> WINED3D_SM4_AOFFIMMI_U_SHIFT; > - aoffimmi.v = (modifier & WINED3D_SM4_AOFFIMMI_V_MASK) >> WINED3D_SM4_AOFFIMMI_V_SHIFT; > - aoffimmi.w = (modifier & WINED3D_SM4_AOFFIMMI_W_MASK) >> WINED3D_SM4_AOFFIMMI_W_SHIFT; > - ins->texel_offset.u = aoffimmi.u; > - ins->texel_offset.v = aoffimmi.v; > - ins->texel_offset.w = aoffimmi.w; > + /* Bit fields are used for sign extension */ > + struct { > + int u : 4; > + int v : 4; > + int w : 4; > + } aoffimmi; Generally, in wined3d, curly braces should be put on their own line, i.e. struct { ... } aoffimmi; The unnamed structs seem to be an exception. The following formatting is acceptable as well: struct { ... } aofimmi; > + > + aoffimmi.u = (modifier & WINED3D_SM4_AOFFIMMI_U_MASK) >> WINED3D_SM4_AOFFIMMI_U_SHIFT; > + aoffimmi.v = (modifier & WINED3D_SM4_AOFFIMMI_V_MASK) >> WINED3D_SM4_AOFFIMMI_V_SHIFT; > + aoffimmi.w = (modifier & WINED3D_SM4_AOFFIMMI_W_MASK) >> WINED3D_SM4_AOFFIMMI_W_SHIFT; > + ins->texel_offset.u = aoffimmi.u; > + ins->texel_offset.v = aoffimmi.v; > + ins->texel_offset.w = aoffimmi.w; > + break; > + }; Please remove stray ";". > + > + case WINED3D_SM5_MODIFIER_RESDIM: > + FIXME("Unhandled modifier 0x%08x (WINED3D_SM5_MODIFIER_RESDIM, res_type=0x%02x).\n", > + modifier, > + (modifier & WINED3D_SM5_RESDIM_RES_TYPE_MASK) >> WINED3D_SM5_RESDIM_RES_TYPE_SHIFT > + ); Please avoid leaving ");" on its own line. > + break; > + > + case WINED3D_SM5_MODIFIER_RETTYPE: > + FIXME("Unhandled modifier 0x%08x (WINED3D_SM5_MODIFIER_RETTYPE, x=0x%01x, y=0x%01x, z=0x%01x, w=0x%01x).\n", > + modifier, > + (modifier & WINED3D_SM5_RETTYPE_X_MASK) >> WINED3D_SM5_RETTYPE_X_SHIFT, > + (modifier & WINED3D_SM5_RETTYPE_Y_MASK) >> WINED3D_SM5_RETTYPE_Y_SHIFT, > + (modifier & WINED3D_SM5_RETTYPE_Z_MASK) >> WINED3D_SM5_RETTYPE_Z_SHIFT, > + (modifier & WINED3D_SM5_RETTYPE_W_MASK) >> WINED3D_SM5_RETTYPE_W_SHIFT > + ); > + break; > + > + default: > + FIXME("Unhandled modifier 0x%08x.\n", modifier); > } > } > > -- > 2.7.4 > > > Thanks for working on this.

1 0