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

Re: USER32: Remove unused variables xoffset, yoffset in SCROLL_TrackScrollBar
by Carlo Bramini 16 Jan '16

16 Jan '16

Hi dear friends, I'm replying here because I have not understood the assigned status as "Formatting" to the attached patch. Thank you very much for your time. Sincerely, Carlo Bramini. >----Messaggio originale---- >Da: Marvin <testbot(a)winehq.org> >Data: 11/01/2016 15.22 >A: "Carlo Bramini"<carlo.bramix(a)libero.it> >Ogg: Re: USER32: Remove unused variables xoffset, yoffset in SCROLL_TrackScrollBar > >Thank you for your contribution to Wine! > >This is an automated notification to let you know that your patch has >been reviewed and its status set to "Formatting". > >This means that there are some formatting issues with your >patch. Either the author's full name, email address, or Signed-off-by >headers are missing, indentation is messed up (usually from editing >with a non-standard tab size), or there are gratuitous formatting >changes to the code. You need to resend a fixed patch. > >If you do not understand the reason for this status, disagree with our >assessment, or are simply not sure how to proceed next, please ask for >clarification by replying to this email.

2 1

Re: [PATCH 1/1] d3drm/tests: Add texture loading tests.
by Aaryaman Vasishta 15 Jan '16

15 Jan '16

On Fri, Jan 15, 2016 at 5:25 PM, Henri Verbeet <hverbeet(a)codeweavers.com> wrote: > + for (i = 0; i < img->palette_size; ++i) > + { > + unsigned int idx = upside_down ? (h - 1) * w - i + (i % w) * > 2 : > How did you come up with that equation? Would be nice to know :) > > + > + hr = IDirect3DRM_LoadTexture(d3drm1, filename, &texture1); > + ok(SUCCEEDED(hr), "Test %u: Failed to load texture, hr %#x.\n", > i, hr); > + d3drm_img = IDirect3DRMTexture_GetImage(texture1); > + todo_wine ok(!!d3drm_img, "Test %u: Failed to get image.\n", i); > Why the '!!' ? How's that better than using a single negation? Other than all that, I've mostly understood the changes you made. Patch looks great :) I'll reply if I have any more queries on the same. Cheers, Aaryaman

2 1

Re: [PATCH] ddraw: Fix use of uninitialized memory.
by Stefan Dösinger 15 Jan '16

15 Jan '16

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Am 2016-01-14 um 20:49 schrieb Thomas Faller: > + if ((desc->dwFlags & DDSD_LPSURFACE) > + && ((desc->ddsCaps.dwCaps & DDSCAPS_ALLOCONLOAD) || !desc->lpSurface)) > desc->dwFlags &= ~DDSD_LPSURFACE; I'd say report this to valgrind as well. If DDSD_LPSURFACE isn't set in the first place then randomly unsetting it doesn't really hurt. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJWmBWcAAoJEN0/YqbEcdMwuuoP/R/psF/dtrnIb0tQH/JP9Ikg kbDNd1NUuwx8QEOKNevopq/ii24whN0VpgJRSKMslY3PvSH6WnWDq9VT3PxSECsc dhfESXJBPSHDfuyzRhVrtwNQvwKrotpUXQ1bIW3AKKsJRr8yOxS3QkYMbt9zMItO YMqy1MP1315Kp6o51LW3iMANipqpmSUfTIjPWrHj7nMKgNTcTbrqOpGHKezxw1u7 ox10hCEwVj5429drUej/Q/nQ1OoAluWAdH0hWZhAF4zi5O+1wwH7I+KwqbL6a/zK 7djvDv+V3EeHxuax3ljwtMoa6Vxwna1vpGSCTHN8N/gNZcFR5v2ne+i+rkGoFQ3V 8u5Lhb1CMaAsTrzBnw+jX2JKSsbpgzFO8nVlDEr2G/cSvf/wq1DaNRpjmFMCExYP 0fs1PzHf4jBubqVChycSbCT1AGEq29s2msn6ucomoTdM1kctRFKjoxPk76iJKE86 mSWBkcmjRSaDBuHt76xjpL9NPEO3CO6lJbtLslEo33kCbsdrv8i+AVGBBvWMf9Oj /bVieoGqk5djdi0bggrhspKGCsVZURYrfpgFv/FIq66Vs7SzEgZoXABoPkjugwtB DLPF2s1WjUqAq0CYWzgLOprvbFI2DPz/VtbaBoJ0GYr3bZJca+ZV8njXsQUMi52b PtFvXa2A4KJOm18CrCm4 =gjon -----END PGP SIGNATURE-----

3 2

Re: shell32/tests: Arrange for the shell_execute{_ex}() calls to set the test line.
by Alexandre Julliard 15 Jan '16

15 Jan '16

Francois Gouget <fgouget(a)codeweavers.com> writes: > This makes it possible to attribute their internal test results to the relevant test_xxx() function. > This implies that they must not make calls that would reset the test line. This is fragile. It would be better to explicitly pass the line number, like most other tests do. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH v2 2/5] wined3d: Recognize SM4 resinfo opcode.
by Henri Verbeet 15 Jan '16

15 Jan '16

On 14 January 2016 at 12:39, Józef Kucia <jkucia(a)codeweavers.com> wrote: > +#define WINED3D_SM4_OPCODESPECIFICCONTROL_SHIFT 11 > +#define WINED3D_SM4_OPCODESPECIFICCONTROL_MASK (0x3u << WINED3D_SM4_OPCODESPECIFICCONTROL_SHIFT) > + I guess the naming derives from shader_sm1.c, but I think I'd prefer something like WINED3D_SM4_INSTRUCTION_FLAGS_MASK. The shader_sm1.c naming for the various masks and shifts is inconsistent in general, I'd like to clean it up to be more like shader_sm4.c at some point. I suspect the mask is actually at least 0xfu (because of WINED3D_SM4_RESOURCE_TYPE_MASK), but we'll find out as soon as we run into an instruction that uses the extra bits. > ins->handler_idx = opcode_info->handler_idx; > - ins->flags = 0; > + ins->flags = (opcode_token & WINED3D_SM4_OPCODESPECIFICCONTROL_MASK) >> WINED3D_SM4_OPCODESPECIFICCONTROL_SHIFT; I think it would be better to do this inside the last else-block of the function. It probably doesn't hurt to have nonsense in the flags field for DCL opcodes, but I think it's best avoided.

1 0

Re: [PATCH] d3d11.idl: Add ID3D11VideoContext interface (try 2)
by Józef Kucia 15 Jan '16

15 Jan '16

On Wed, Jan 13, 2016 at 8:08 AM, Andrey Turkin <andrey.turkin(a)gmail.com> wrote: > UINT Usage:1; It would preferably be written as "UINT Usage : 1;". This applies to other bit fields. > + void VideoProcessorSetOutputConstriction( > + [in] ID3D11VideoProcessor *pVideoProcessor, > + BOOL Enable, > + SIZE Size); > + void VideoProcessorSetOutputStereoMode( > + [in] ID3D11VideoProcessor *pVideoProcessor, > + BOOL Enable); The Enable and Size parameters should be annotated as input parameters. Ideally, the method parameters would be renamed and written in lower_case_with_underscores.

4 5

Re: [PATCH v2] usp10: rewrite ScriptXtoCP
by Marvin 15 Jan '16

15 Jan '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=19719 Your paranoid android. === w1064 (32 bit usp10) === usp10.c:1244: Test failed: 0: uJustification incorrect (0) usp10.c:1244: Test failed: 1: uJustification incorrect (0) usp10.c:1244: Test failed: 2: uJustification incorrect (0) usp10.c:1244: Test failed: 3: uJustification incorrect (0) usp10.c:1244: Test failed: 4: uJustification incorrect (0) usp10.c:1244: Test failed: 5: uJustification incorrect (0) usp10.c:1244: Test failed: 6: uJustification incorrect (0) usp10.c:1244: Test failed: 7: uJustification incorrect (0) usp10.c:1244: Test failed: 8: uJustification incorrect (0) usp10.c:1244: Test failed: 9: uJustification incorrect (0) usp10.c:1260: Test failed: 3: uJustification incorrect (0) usp10.c:1260: Test failed: 5: fDiacritic incorrect (1) usp10.c:1260: Test failed: 5: fZeroWidth incorrect (1) usp10.c:1260: Test failed: 15: fDiacritic incorrect (1) usp10.c:1260: Test failed: 15: fZeroWidth incorrect (1)

1 0

Re: [PATCH] usp10: rewrite ScriptXtoCP
by Huw Davies 15 Jan '16

15 Jan '16

Hi Aric, Just some comments to keep the ball rolling on this. In general I can now at least understand the algorithm, so it's a vast improvement over the one it replaces :-) On Mon, Jan 04, 2016 at 07:52:59AM -0600, Aric Stewart wrote: > diff --git a/dlls/usp10/usp10.c b/dlls/usp10/usp10.c > +/* Count the number of characters in a cluster */ > +static inline int get_cluster_charsize(const WORD *pwLogClust, int cChars, int index) > +{ > + int size = 0; > + int i; > + for (i = 0; i < cChars; i++) > + if (pwLogClust[i] == index) size++; > + return size; Does it make sense to break out of this loop if (size && pwLogClust[i] != index) ? > +} > + > +/* > + To handle multi-glyph clusters we need to find all the glyphs that are > + represented in by the cluster. This involves finding the glyph whose "in by" > + index is the cluster index as well as whose glyph indices are greater than > + our cluster index but not part of a new cluster. > + > + Then we sum all those glyph's advances. glyphs' > +*/ > +static inline int get_cluster_advance(const int* piAdvance, const SCRIPT_VISATTR *psva, const WORD *pwLogClust, int cGlyphs, int cChars, int cluster, int direction) Some of these lines are getting awfully long. > +{ > + int glyph_start; > + int glyph_end; > + int i, advance; > + > + if (direction > 0) > + i = 0; > + else > + i = (cChars - 1); > + > + for (glyph_start = -1, glyph_end = -1; i < cChars && i >= 0 && (glyph_start < 0 || glyph_end < 0); i+=direction) > + { > + if (glyph_start < 0 && pwLogClust[i] != cluster) continue; > + if (pwLogClust[i] == cluster && glyph_start < 0) glyph_start = pwLogClust[i]; > + if (glyph_start >= 0 && glyph_end < 0 && pwLogClust[i] != cluster) glyph_end = pwLogClust[i]; > + } > + if (glyph_end < 0) > + { > + if (direction > 0) > + glyph_end = cGlyphs; > + else /* Don't understand multi-glyph reversed clusters yet, do they occur for real? */ Should we add a FIXME for this? > + glyph_end = glyph_start + 1; > + } > + > + /* Check for fClusterStart, finding this generally would mean a malformed set of data */ > + for (i = glyph_start+1; i< glyph_end; i++) > + { > + if (psva[i].fClusterStart) > + { > + glyph_end = i; > + break; > + } > + } > + > + for ( advance = 0, i = glyph_start; i < glyph_end; i++) Odd leading space before advance. > + advance += piAdvance[i]; > + > + return advance; > +} > + > + > /*********************************************************************** > * ScriptXtoCP (USP10.@) > * > + * Basic algorithm : > + * use piAdvance to find the cluster we are looking at > + * Find the character that is the first character of the cluster > + * That is our base piCP > + * If the script snaps to cluster boundries (Hebrew, Indic, Thai) then we are good > + * Otherwise if the cluster is larger than 1 glyph we need to determin how far through the cluster determine. > + * to advance the cursor. > */ > HRESULT WINAPI ScriptXtoCP(int iX, > int cChars, > @@ -2587,16 +2655,10 @@ HRESULT WINAPI ScriptXtoCP(int iX, > int *piCP, > int *piTrailing) > { > - int item; > - float iPosX; > - float iLastPosX; > - int iSpecial = -1; > - int iCluster = -1; > - int clust_size = 1; > - int cjump = 0; > - int advance; > - float special_size = 0.0; > int direction = 1; > + int iPosX; > + int i; > + int glyph_index, cluster_index; > > TRACE("(%d,%d,%d,%p,%p,%p,%p,%p,%p)\n", > iX, cChars, cGlyphs, pwLogClust, psva, piAdvance, > @@ -2605,6 +2667,7 @@ HRESULT WINAPI ScriptXtoCP(int iX, > if (psa->fRTL && ! psa->fLogicalOrder) > direction = -1; > > + /* Looking for non-reversed clusters in a reversed string */ > if (direction<0) > { > int max_clust = pwLogClust[0]; > @@ -2616,14 +2679,15 @@ HRESULT WINAPI ScriptXtoCP(int iX, > return S_OK; > } > > - for (item=0; item < cChars; item++) > - if (pwLogClust[item] > max_clust) > + for (i=0; i< cChars; i++) > + if (pwLogClust[i] > max_clust) > { > ERR("We do not handle non reversed clusters properly\n"); > break; > } > } > > + /* Handle an iX < 0 */ > if (iX < 0) > { > *piCP = -1; > @@ -2631,102 +2695,134 @@ HRESULT WINAPI ScriptXtoCP(int iX, > return S_OK; > } > > - iPosX = iLastPosX = 0; > + /* find the glyph_index based in iX */ > if (direction > 0) > - item = 0; > + { > + for (glyph_index = -1, iPosX = iX; iPosX >=0 && glyph_index < cGlyphs; iPosX -= piAdvance[glyph_index+1], glyph_index++) > + ; > + } > else > - item = cChars - 1; > - for (; iPosX <= iX && item < cChars && item >= 0; item+=direction) > - { > - iLastPosX = iPosX; > - if (iSpecial == -1 && > - (iCluster == -1 || > - (iCluster != -1 && > - ((direction > 0 && iCluster+clust_size <= item) || > - (direction < 0 && iCluster-clust_size >= item)) > - ) > - ) > - ) > + { > + for (glyph_index = -1, iPosX = iX; iPosX > 0 && glyph_index < cGlyphs; iPosX -= piAdvance[glyph_index+1], glyph_index++) > + ; > + } > + > + TRACE("iPosX %i -> glyph_index %i (%i)\n", iPosX, glyph_index, cGlyphs); > + > + *piTrailing = 0; > + if (glyph_index >= 0 && glyph_index < cGlyphs) > + { > + /* find the cluster */ > + if (direction > 0 ) > + for (i = 0, cluster_index = pwLogClust[0]; i < cChars && pwLogClust[i] <= glyph_index; cluster_index=pwLogClust[i++]) > + ; > + else > + for (i = 0, cluster_index = pwLogClust[0]; i < cChars && pwLogClust[i] >= glyph_index; cluster_index=pwLogClust[i++]) > + ; > + > + TRACE("cluster_index %i\n", cluster_index); > + > + if (direction < 0 && iPosX >= 0 && glyph_index != cluster_index) > { > - int check; > - int clust = pwLogClust[item]; > + /* We are off the end of the string */ > + *piCP = -1; > + *piTrailing = 1; > + return S_OK; > + } > > - iCluster = -1; > - cjump = 0; > - clust_size = get_cluster_size(pwLogClust, cChars, item, direction, > - &iCluster, &check); > - advance = get_glyph_cluster_advance(piAdvance, psva, pwLogClust, cGlyphs, cChars, clust, direction); > + /* find the first character of the cluster */ > + for (i = 0; i < cChars && pwLogClust[i] != cluster_index; i++) > + ; > > - if (check >= cChars && direction > 0) > - { > - int glyph; > - for (glyph = clust; glyph < cGlyphs; glyph++) > - special_size += get_glyph_cluster_advance(piAdvance, psva, pwLogClust, cGlyphs, cChars, glyph, direction); > - iSpecial = item; > - special_size /= (cChars - item); > - iPosX += special_size; > - } > - else > + TRACE("first char index %i\n",i); > + if (scriptInformation[psa->eScript].props.fNeedsCaretInfo) > + { > + /* Check trailing */ > + if (glyph_index != cluster_index || > + (direction > 0 && abs(iPosX) <= (piAdvance[glyph_index] / 2)) || > + (direction < 0 && abs(iPosX) >= (piAdvance[glyph_index] / 2))) > + *piTrailing = get_cluster_charsize(pwLogClust,cChars,cluster_index); > + } > + else > + { > + int cluster_size; > + > + cluster_size = get_cluster_charsize(pwLogClust, cChars, cluster_index); > + > + if (cluster_size > 1) > { > - if (scriptInformation[psa->eScript].props.fNeedsCaretInfo) > + /* Be part way through the glyph cluster based on size and position */ > + int cluster_advance = get_cluster_advance(piAdvance, psva, pwLogClust, cGlyphs, cChars, cluster_index, direction); > + double cluster_part_width = cluster_advance / (float)cluster_size; > + double adv; > + int part_index; > + > + /* back up to the beginning of the cluster */ > + for (adv = iPosX, part_index = cluster_index; part_index <= glyph_index; part_index++) > + adv += piAdvance[part_index]; > + if (adv > iX) adv = iX; > + > + TRACE("Multi-char cluster, no snap\n"); > + TRACE("cluster size %i, pre-cluster iPosX %f\n",cluster_size, adv); > + TRACE("advance %i divides into %f per char\n", cluster_advance, cluster_part_width); > + if (direction > 0) > { > - if (!cjump) > - iPosX += advance; > - cjump++; > + for (part_index = 0; adv >= 0; adv-=cluster_part_width, part_index++) > + ; > + if (part_index) part_index--; > } > else > - iPosX += advance / (float)clust_size; > + { > + for (part_index = 0; adv > 0; adv-=cluster_part_width, part_index++) > + ; > + if (part_index > cluster_size) > + { > + adv += cluster_part_width; > + part_index=cluster_size; > + } > + } > + > + TRACE("base_char %i part_index %i, leftover advance %f\n",i, part_index, adv); > + > + if (direction > 0) > + i += part_index; > + else > + i += (cluster_size - part_index); > + > + /* Check trailing */ > + if ((direction > 0 && fabs(adv) <= (cluster_part_width / 2.0)) || > + (direction < 0 && adv && fabs(adv) >= (cluster_part_width / 2.0))) > + *piTrailing = 1; > } > - } > - else if (iSpecial != -1) > - iPosX += special_size; > - else /* (iCluster != -1) */ > - { > - int adv = get_glyph_cluster_advance(piAdvance, psva, pwLogClust, cGlyphs, cChars, pwLogClust[iCluster], direction); > - if (scriptInformation[psa->eScript].props.fNeedsCaretInfo) > + else > { > - if (!cjump) > - iPosX += adv; > - cjump++; > + /* Check trailing */ > + if ((direction > 0 && abs(iPosX) <= (piAdvance[glyph_index] / 2)) || > + (direction < 0 && abs(iPosX) >= (piAdvance[glyph_index] / 2))) > + *piTrailing = 1; > } > - else > - iPosX += adv / (float)clust_size; > } > } > - > - if (direction > 0) > - { > - if (iPosX > iX) > - item--; > - if (item < cChars && ((iPosX - iLastPosX) / 2.0) + iX >= iPosX) > - { > - if (scriptInformation[psa->eScript].props.fNeedsCaretInfo && clust_size > 1) > - item+=(clust_size-1); > - *piTrailing = 1; > - } > - else > - *piTrailing = 0; > - } > else > { > - if (iX == iLastPosX) > - item++; > - if (iX >= iLastPosX && iX <= iPosX) > - item++; > + TRACE("Point falls outside of string\n"); > + if (glyph_index < 0) > + i = cChars-1; > + else /* (glyph_index >= cGlyphs) */ > + i = cChars; > > - if (iLastPosX == iX) > - *piTrailing = 0; > - else if (item < 0 || ((iLastPosX - iPosX) / 2.0) + iX <= iLastPosX) > + /* If not snaping in the reverse direction (such as Hebrew) Then 0 > + point flow to the next character */ > + if (direction < 0) > { > - if (scriptInformation[psa->eScript].props.fNeedsCaretInfo && clust_size > 1) > - item-=(clust_size-1); > - *piTrailing = 1; > + if (!scriptInformation[psa->eScript].props.fNeedsCaretInfo && abs(iPosX) == piAdvance[glyph_index]) > + i++; > + else > + *piTrailing = 1; > } > - else > - *piTrailing = 0; > } > > - *piCP = item; > + *piCP = i; > > TRACE("*piCP=%d\n", *piCP); > TRACE("*piTrailing=%d\n", *piTrailing); > >

2 1

Re: [v4 1/2] ole32: OleGetClipboard(): zero output interface pointer on error ole32/tests: OleGetClipboard fails on locked clipboard; out iface ptr is zeroed
by Huw Davies 15 Jan '16

15 Jan '16

On Fri, Jan 15, 2016 at 02:33:07PM +0300, Paul Gofman wrote: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/ole32/clipboard.c | 1 + > dlls/ole32/tests/clipboard.c | 20 ++++++++++++++++++++ > 2 files changed, 21 insertions(+) One more thing (sorry, I should have noticed earlier), in the subject don't add a second bit referring to the tests. For this patch something like: ole32: OleGetClipboard(): zero output interface pointer on error. will be fine. While you're at it, replace the word 'natively' with 'should' in the second patch's subject. Thanks, Huw.

1 0

Re: [PATCH v2] ole32/tests: OleGetClipboard natively succeeds when COM/OLE is not inited
by Huw Davies 15 Jan '16

15 Jan '16

On Fri, Jan 15, 2016 at 01:56:29PM +0300, Paul Gofman wrote: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/ole32/tests/clipboard.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/dlls/ole32/tests/clipboard.c b/dlls/ole32/tests/clipboard.c > index 02097ea..c6ef736 100644 > --- a/dlls/ole32/tests/clipboard.c > +++ b/dlls/ole32/tests/clipboard.c > @@ -455,6 +455,17 @@ static HRESULT DataObjectImpl_CreateComplex(LPDATAOBJECT *lplpdataobj) > return S_OK; > } > > +static void test_get_clipboard_unitialized(void) > +{ > + HRESULT hr; > + IDataObject *pDObj; > + > + pDObj = (IDataObject *)0xdeadbeef; > + hr = OleGetClipboard(&pDObj); > + todo_wine ok(hr == S_OK, "OleGetClipboard() got 0x%08x instead of 0x%08x\n", hr, S_OK); > + if (pDObj && pDObj != (IDataObject *)0xdeadbeef) IUnknown_Release((IUnknown*)pDObj); Use IDataObject_Release to avoid a cast. Huw.

1 0