March 2017 - Wine-Devel - list.winehq.org

Re: [PATCH] oleaut32: Implement GetAltMonthNames()

by Jacek Caban

Hi Nikolay, On 3/24/17 7:32 PM, Nikolay Sivov wrote: > diff --git a/dlls/oleaut32/oleaut.c b/dlls/oleaut32/oleaut.c > index f9734e6d78..816b18ce35 100644 > --- a/dlls/oleaut32/oleaut.c > +++ b/dlls/oleaut32/oleaut.c > @@ -935,3 +935,109 @@ HCURSOR WINAPI OleIconToCursor( HINSTANCE hinstExe, HICON hIcon) > /* FIXME: make an extended conversation from HICON to HCURSOR */ > return CopyCursor(hIcon); > } > + > +BOOL WINAPI GetAltMonthNames(LCID lcid, LPOLESTR **str) Shouldn't it return HRESULT? Thanks, Jacek

March 25, 2017

Re: cmd: Adjust CMD_LIST to a tree-like structure and implement command chains

by Stefan Dösinger

> Am 24.03.2017 um 17:26 schrieb Flávio J. Saraiva <flaviojs2005(a)gmail.com>: > > If what winehq untimately cares about is not having regressions, then > the best approach here is to split it into logical bits (only passing > tests at the end of the patch set) and not try to minimize the changes > in the parts i'm being forced to change. > > This means that if one patch gets rejected then the whole set is also rejected. > I guess this wouldn't be a problem? > It is a potential problem for bisects. Let's assume I am bisecting a direct3d regression in an app that also uses cmd.exe. Somewhere between the good and bad revisions for the d3d problem there is a revision where the app fails because of cmd. That said, this sometimes happens and it's solvable with some extra work during bisects (e.g. manually applying the fix for cmd for revisions that are broken in a way I don't care). But this is one of the reasons why e.g. having a temporary build failure is a no-go.

March 25, 2017

Re: [PATCH v2 1/5] user32: Fix handling of internal messages.

by Alexandre Julliard

Zebediah Figura <z.figura12(a)gmail.com> writes: > This patch adds a new flag and a server reply, allowing GetQueueStatus > to ignore internal messages, and MsgWaitForMultipleObjects to process > internal messages until a real sent message is found. There's a lot of code duplication in there, and adding private flags to public headers is discouraged. There has to be a better way of doing this, or else it would need a stronger justification than a couple of EnableWindow tests. -- Alexandre Julliard julliard(a)winehq.org

March 24, 2017

Re: cmd: Adjust CMD_LIST to a tree-like structure and implement command chains

by Flávio J. Saraiva

2017-03-23 23:15 GMT+00:00 Henri Verbeet <hverbeet(a)gmail.com>: > You can, but that still potentially regresses functionality for > applications, which is what we ultimately care about. It may be > justified in some situations, but if at all possible it's still best > avoided. If what winehq untimately cares about is not having regressions, then the best approach here is to split it into logical bits (only passing tests at the end of the patch set) and not try to minimize the changes in the parts i'm being forced to change. This means that if one patch gets rejected then the whole set is also rejected. I guess this wouldn't be a problem? I'll proceed like this for now and review this slowly. In the current patch the only part that might contain an untested regression is the 'CALL FOR' part (that's why it has a FIXME). Flávio J. Saraiva

March 24, 2017

Re: [PATCH] comctl32/taskdialog: Implement buttons.

by Fabian Maurer

> > -static const UINT DIALOG_MIN_WIDTH = 180; > > +static const UINT DIALOG_MIN_WIDTH = 240; > > > > static const UINT DIALOG_SPACING = 5; > > > > -static const UINT DIALOG_BUTTON_WIDTH = 50; > > -static const UINT DIALOG_BUTTON_HEIGHT = 14; > > +static const UINT DIALOG_BUTTON_WIDTH_MIN = 45; > > +static const UINT DIALOG_BUTTON_HEIGHT = 13; > > This was taken from MSGBOX template, what's a point in making it different? I just tested it on my Win7 box and tried to get it to look as similar as possible. Where do the MSGBOX template values come from? I can remove that change, if those values are better. > Did you test that buttons are supposed to support multiple lines of text? Normal buttons don't allow multiple lines of text. But we only use the right side of the rectangle anyways, so it works as expected. Problem is, on windows and on wine the buttons clips the text when it gets too long, so I'm not fully it's the correct way to implement this. Should I change this? There's also the fact that buttons get larger in height on windows, even if the text is a single line only. Should I reproduce this? Moreover, wine displays newlines as boxes while this isn't the case on windows. Not sure how to deal with this though. > > static unsigned int taskdialog_add_control(struct > > taskdialog_template_desc *desc, WORD id, const WCHAR *class,> > > - HINSTANCE hInstance, const WCHAR *text, short x, short y, short > > cx, short cy) + DWORD style, HINSTANCE hInstance, const WCHAR > > *text, short x, short y, short cx, short cy)> > > { > > > > struct taskdialog_control *control = Alloc(sizeof(*control)); > > unsigned int size, class_size, text_size; > > > > @@ -117,7 +167,7 @@ static unsigned int taskdialog_add_control(struct > > taskdialog_template_desc *desc> > > control->template = template = Alloc(size); > > control->template_size = size; > > > > - template->style = WS_VISIBLE; > > + template->style = WS_VISIBLE | WS_CHILD | style; > > You don't use styles in this patch, this is unnecessary change. I'm using BS_PUSHBUTTON for the buttons, is this unnecessary? > > > template->dwExtendedStyle = 0; > > template->x = x; > > template->y = y; > > > > @@ -136,78 +186,126 @@ static unsigned int taskdialog_add_control(struct > > taskdialog_template_desc *desc> > > static unsigned int taskdialog_add_main_instruction(struct > > taskdialog_template_desc *desc) { > > > > - RECT rect = { 0, 0, desc->dialog_width - DIALOG_SPACING * 2, 0}; /* > > padding left and right of the control */ - const WCHAR *textW = NULL; > > - unsigned int size, length; > > - HFONT oldfont; > > - HDC hdc; > > + unsigned int size; > > + RECT rect; > > > > if (!desc->taskconfig->pszMainInstruction) > > > > return 0; > > > > - if (IS_INTRESOURCE(desc->taskconfig->pszMainInstruction)) > > - { > > - if (!(length = LoadStringW(desc->taskconfig->hInstance, > > (UINT_PTR)desc->taskconfig->pszMainInstruction, - (WCHAR > > *)&textW, 0))) > > - { > > - WARN("Failed to load main instruction text\n"); > > - return 0; > > - } > > - } > > - else > > - { > > - textW = desc->taskconfig->pszMainInstruction; > > - length = strlenW(textW); > > - } > > - > > - hdc = GetDC(0); > > - oldfont = SelectObject(hdc, desc->font); > > - > > - dialogunits_to_pixels(desc, &rect.right, NULL); > > - DrawTextW(hdc, textW, length, &rect, DT_LEFT | DT_EXPANDTABS | > > DT_CALCRECT | DT_WORDBREAK); - pixels_to_dialogunits(desc, > > &rect.right, &rect.bottom); > > - > > - SelectObject(hdc, oldfont); > > - ReleaseDC(0, hdc); > > + rect = text_get_rect(desc, desc->taskconfig->pszMainInstruction); > > > > - size = taskdialog_add_control(desc, ID_MAIN_INSTRUCTION, WC_STATICW, > > desc->taskconfig->hInstance, + size = taskdialog_add_control(desc, > > ID_MAIN_INSTRUCTION, WC_STATICW, 0, desc->taskconfig->hInstance,> > > desc->taskconfig->pszMainInstruction, DIALOG_SPACING, > > desc->dialog_height, > > rect.right, rect.bottom); > > > > - desc->dialog_height += rect.bottom; > > + desc->dialog_height += rect.bottom + DIALOG_SPACING; > > > > return size; > > > > } > > > > -static unsigned int taskdialog_add_common_buttons(struct > > taskdialog_template_desc *desc) +static button_info make_button(struct > > taskdialog_template_desc *desc, int id, const WCHAR *text) +{ > > + RECT rect; > > + button_info button; > > + > > + button.id = id; > > + button.text = text; > > + rect = text_get_rect(desc, text); > > + button.width = rect.right + 10; > > + if (button.width < DIALOG_BUTTON_WIDTH_MIN) > > + button.width = DIALOG_BUTTON_WIDTH_MIN; > > + > > + return button; > > +} > > This does not handle "text" correctly. Sorry, but I don't see the problem here. Do you mind explaining? In both text_get_rect and later on taskdialog_add_control both text or a resource-ID are allowed. > > +static unsigned int taskdialog_add_buttons(struct > > taskdialog_template_desc *desc)> > > { > > > > - short button_x = desc->dialog_width - DIALOG_BUTTON_WIDTH - > > DIALOG_SPACING;> > > DWORD flags = desc->taskconfig->dwCommonButtons; > > > > + const TASKDIALOGCONFIG *taskconfig = desc->taskconfig; > > + UINT alignment = DIALOG_SPACING_BUTTONS_LEFT; /* minimum distance > > from the left dialog border */ + UINT location_x; > > + BOOL first_row = TRUE; > > + button_info *buttons; > > + int count = 0; > > + int i; > > > > unsigned int size = 0; > > > > -#define TASKDIALOG_ADD_COMMON_BUTTON(id) \ > > - do { \ > > - size += taskdialog_add_control(desc, ID##id, WC_BUTTONW, > > COMCTL32_hModule, MAKEINTRESOURCEW(IDS_BUTTON_##id), \ - > > button_x, desc->dialog_height + DIALOG_SPACING, DIALOG_BUTTON_WIDTH, > > DIALOG_BUTTON_HEIGHT); \ - button_x -= DIALOG_BUTTON_WIDTH + > > DIALOG_SPACING; \ > > - } while(0) > > - if (flags & TDCBF_CLOSE_BUTTON) > > - TASKDIALOG_ADD_COMMON_BUTTON(CLOSE); > > - if (flags & TDCBF_CANCEL_BUTTON) > > - TASKDIALOG_ADD_COMMON_BUTTON(CANCEL); > > - if (flags & TDCBF_RETRY_BUTTON) > > - TASKDIALOG_ADD_COMMON_BUTTON(RETRY); > > - if (flags & TDCBF_NO_BUTTON) > > - TASKDIALOG_ADD_COMMON_BUTTON(NO); > > - if (flags & TDCBF_YES_BUTTON) > > - TASKDIALOG_ADD_COMMON_BUTTON(YES); > > + /* Allocate enough memory for the custom and the default buttons */ > > + if (taskconfig->cButtons && taskconfig->pButtons) > > + buttons = HeapAlloc(GetProcessHeap(), 0, (taskconfig->cButtons + > > 6) * sizeof(button_info)); + else > > + { > > + buttons = HeapAlloc(GetProcessHeap(), 0, 6 * > > sizeof(button_info)); > > + } > > + > > + /* Custom buttons */ > > + if (taskconfig->cButtons && taskconfig->pButtons) > > + { > > + for (i = 0; i < taskconfig->cButtons; i++) > > + { > > + buttons[count++] = make_button(desc, > > taskconfig->pButtons[i].nButtonID, > > taskconfig->pButtons[i].pszButtonText); + } > > + } > > + > > + /* Default buttons */ > > > > if (flags & TDCBF_OK_BUTTON) > > > > - TASKDIALOG_ADD_COMMON_BUTTON(OK); > > - /* Always add OK button */ > > - if (list_empty(&desc->controls)) > > - TASKDIALOG_ADD_COMMON_BUTTON(OK); > > -#undef TASKDIALOG_ADD_COMMON_BUTTON > > - > > - /* make room for common buttons row */ > > - desc->dialog_height += DIALOG_BUTTON_HEIGHT + 2 * DIALOG_SPACING; > > + buttons[count++] = make_button(desc, IDOK, > > MAKEINTRESOURCEW(IDS_BUTTON_OK)); + if (flags & TDCBF_YES_BUTTON) > > + buttons[count++] = make_button(desc, IDYES, > > MAKEINTRESOURCEW(IDS_BUTTON_YES)); + if (flags & TDCBF_NO_BUTTON) > > + buttons[count++] = make_button(desc, IDNO, > > MAKEINTRESOURCEW(IDS_BUTTON_NO)); + if (flags & TDCBF_RETRY_BUTTON) > > + buttons[count++] = make_button(desc, IDRETRY, > > MAKEINTRESOURCEW(IDS_BUTTON_RETRY)); + if (flags & > > TDCBF_CANCEL_BUTTON) > > + buttons[count++] = make_button(desc, IDCANCEL, > > MAKEINTRESOURCEW(IDS_BUTTON_CANCEL)); + if (flags & > > TDCBF_CLOSE_BUTTON) > > + buttons[count++] = make_button(desc, IDCLOSE, > > MAKEINTRESOURCEW(IDS_BUTTON_CLOSE)); + > > + /* There must be at least one button */ > > + if (count == 0) > > + buttons[count++] = make_button(desc, IDOK, > > MAKEINTRESOURCEW(IDS_BUTTON_OK)); + > > + /* Position buttons */ > > + location_x = alignment; > > + for (i = 0; i < count; i++) > > + { > > + /* When beginning new row, align the first */ > > + if (location_x + buttons[i].width + DIALOG_SPACING_BUTTON_W > > > desc->dialog_width) + { > > + if (first_row) > > + { > > + int diff = desc->dialog_width - location_x; > > + > > + first_row = FALSE; > > + for (int j = 0; j < i; j++) /* Align first row to the > > right */ + buttons[j].x += diff; > > + alignment = buttons[0].x; /* left-align all coming rows > > to the first row */ + } > > + location_x = alignment; > > + desc->dialog_height += DIALOG_BUTTON_HEIGHT + > > DIALOG_SPACING_BUTTON_H; + } > > + > > + buttons[i].x = location_x; > > + buttons[i].y = desc->dialog_height; > > + > > + location_x += buttons[i].width + DIALOG_SPACING_BUTTON_W; > > + } > > + if (first_row) /* Always align first row to the right */ > > + { > > + int diff = desc->dialog_width - (buttons[count - 1].x + > > buttons[count - 1].width + DIALOG_SPACING_BUTTON_W); + for (int i > > = 0; i < count; i++) > > + buttons[i].x += diff; > > + } > > I don't like this. Why do you need temporary array, and iterating twice > seems excessive. The problem is, a later button might influence the position of the first button. That's why I created them as temporary array and then change their position once I know how. I also could iterate the loop twice I guess, but that sounded a bit ugly, too. If you have a better idea to address this problem, I'd be happy to hear it. > > + for (i = 0; i < count; i++) > > + { > > + size += taskdialog_add_control(desc, buttons[i].id, WC_BUTTONW, > > BS_PUSHBUTTON, COMCTL32_hModule, + buttons[i].text, > > buttons[i].x, buttons[i].y, buttons[i].width, DIALOG_BUTTON_HEIGHT); + > > } > > This won't work for custom buttons. Right, I missed that, thanks for pointing it out. > > - size += taskdialog_add_common_buttons(&desc); > > + size += taskdialog_add_buttons(&desc); > > I'd rather have it separated if possible. I'll do some testing myself, > before commenting further. I see. Just figured it would make sense to handle all buttons in one function, since they are aligned together - if no command-links are used. Is there a reason you want it separated? Regards, Fabian Maurer

March 24, 2017

Re: [PATCH 2/3] d3dx9: Support relative addressing in preshader.

by Paul Gofman

On 03/24/2017 05:10 PM, Matteo Bruni wrote: > 2017-03-24 2:16 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: >> >> Sorry, I messed up things actually, base register actually can come from >> temporary (register) table also, which happens in the test. So I will >> introduce extra structure for registers. > So I had a look at my stuff. As you know it has got very little and > specific testing and is most likely missing some cases but I see that: > - for relative addressing I accept any register type as base but > expect only temporary registers as index (i.e. I look for "flags" == 1 > and then for the "base" fields only when parsing TEMP registers) > - for CONST registers the "flags" != 0 field has a different meaning. > In that case it is an offset in the IMMED table and there is no > additional "base" register type + index in the effect bytecode, so > it's indexed like imm["flags" + cidx]. > What is seen from my tests for relative addressing (all these cases below are present in the test currently there in the last patch I sent, though I will add another test in the updated patch which covers more cases, and also has a non-zero absolute offset, which helped me to discovered and fix the bug): 1. Temporary register or constant float (e. g. c0) register can be used as base register in relative addressing. Temporary register is used by compiler when it needs to do some precompute for the index value, like rounding, multiplying by stride for structure or any other arithmetic with indexes, otherwise cX registers are used directly. 2. If the value being indexed is 4x vectors aligned, the main register being indexed is cX, it is directly indexed by base register; 3. If the value we are indexing into is some vector, or matrix, or anything not 4x aligned, compiler generates a code doing the indexing through a 'dot' instructions with base register indexing immediate constants to dot with. This is basically present in the test I sent (doing a trace with +d3dx will show preshaders with these relative addressing types, the string to easily find the code in the output is 'imm['). in the next update I will include another test which is a bit more complicated in terms of this 'dot' arithmetic. So unless I am missing something I don't see yet how flags have a different meaning for const or immed registers, it looks very straightforward and matches the test.

March 24, 2017

Re: [PATCH 2/3] d3dx9: Support relative addressing in preshader.

by Matteo Bruni

2017-03-24 2:16 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > On 03/24/2017 03:31 AM, Matteo Bruni wrote: >> >> 2017-03-24 0:39 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: >>> >>> On 03/24/2017 01:57 AM, Matteo Bruni wrote: >>>> >>>> 2017-03-23 16:06 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: >>>>> >>>>> Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> >>>>> --- >>>>> dlls/d3dx9_36/preshader.c | 103 >>>>> ++++++++++++++++++++++++++++++++++++------- >>>>> dlls/d3dx9_36/tests/effect.c | 38 ++++++++-------- >>>>> 2 files changed, 106 insertions(+), 35 deletions(-) >>>>> >>>>> diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c >>>>> index 9aedc99..2974ffd 100644 >>>>> --- a/dlls/d3dx9_36/preshader.c >>>>> +++ b/dlls/d3dx9_36/preshader.c >>>>> @@ -191,6 +191,8 @@ struct d3dx_pres_operand >>>>> /* offset is component index, not register index, e. g. >>>>> offset for component c3.y is 13 (3 * 4 + 1) */ >>>>> unsigned int offset; >>>>> + enum pres_reg_tables base_reg_table; >>>>> + unsigned int base_reg_offset; >>>>> }; >>>> >>>> It would be nicer to create a struct for storing together table and >>>> offset and to pass it around to the various functions taking both >>>> arguments, like parse_pres_reg(). >>>> It should probably be a separate patch before this one. >>> >>> Actually I can't guess the case when base_reg_table would be not a >>> floating >>> constant preshader input. Maybe I just remove this variable and check >>> with >>> FIXME during parsing? >> >> Sure, that's fine. Still the struct seems useful for parameter >> passing, if not right there in struct d3dx_pres_operand. >> > Sorry, I messed up things actually, base register actually can come from > temporary (register) table also, which happens in the test. So I will > introduce extra structure for registers. So I had a look at my stuff. As you know it has got very little and specific testing and is most likely missing some cases but I see that: - for relative addressing I accept any register type as base but expect only temporary registers as index (i.e. I look for "flags" == 1 and then for the "base" fields only when parsing TEMP registers) - for CONST registers the "flags" != 0 field has a different meaning. In that case it is an offset in the IMMED table and there is no additional "base" register type + index in the effect bytecode, so it's indexed like imm["flags" + cidx]. Unfortunately I don't think I have any effect with the latter feature around. Supporting that should probably be a separate patch but try to see if you can replicate that. >>>>> static double exec_get_arg(struct d3dx_regstore *rs, const struct >>>>> d3dx_pres_operand *opr, unsigned int comp) >>>>> { >>>>> - if (!regstore_is_val_set_reg(rs, opr->table, (opr->offset + comp) >>>>> / >>>>> table_info[opr->table].reg_component_count)) >>>>> - WARN("Using uninitialized input, table %u, offset %u.\n", >>>>> opr->table, opr->offset + comp); >>>>> + unsigned int base_offset, index; >>>>> + >>>>> + if (opr->base_reg_table == PRES_REGTAB_COUNT) >>>>> + index = 0; >>>>> + else >>>>> + index = (int)exec_get_reg_value(rs, opr->base_reg_table, >>>>> opr->base_reg_offset); >>>> >>>> I think it's better with lrint() in place of the cast. >>> >>> I just want to note that whenever the input is from floating values FX >>> compiler generates preshader instructions which do the rounding, and >>> skips >>> that if the base register comes from integers. Do you think we need to do >>> extra rounding in this case? >> >> Ah, you're right, I somehow overlooked that. Actually I think >> doublechecking (e.g. with a FIXME) that the source is really integer >> would be useful. If it isn't too much of a mess it would be nice to >> avoid the roundtrip to doubles for the index entirely. > > Source of the base register value seems not so straightforward to check. > The input table always has float format. To do such check, we need to find > which parameter the register is initialized from, we don't have such a link > now. Right, that's hard to do in the current architecture. Also I confused this with the IMMED relative addressing I mentioned above, for those my code expects the input CONST register to be mapped to an INT parameter. That's only relevant in my architecture since I don't fill all the input registers before executing the preshader but go fetch them "on-the-fly" for each instruction. That is also clearly worse than your solution, of course. Long story short, just ignore this. > Besides, using float register as index seems totally ok in effect, > compiler generates rounding code for this case. Moreover, base register used > in instruction may come from temporary ("r") registers and be a result of > some operations, like rounding or multiplication when indexing anything with > stride other than 4xfloat. Right as well. I don't know, lrintf() seems like a good idea still. > Or could it be you had in mind not checking the source of register > value, but rather that the value is actually an integer (fractional part is > zero)? That might be a bit fragile (because of potential numerical accuracy troubles), although as you noticed the compiler seems to go to great lengths to make sure the value is exactly an integer. Maybe doing that kind of check only in a if (WARN_ON(d3dx)) might make sense as it would be somewhat surprising and it might reveal something else going on but it doesn't seem worth it to me in general.

March 24, 2017

Re: [PATCH v2] winex11: Use GLX_OML_sync_control to synchronise buffer swaps with X11DRV_FLUSH_GL_DRAWABLE requests.

by Henri Verbeet

On 24 March 2017 at 13:02, Jacek Caban <jacek(a)codeweavers.com> wrote: > On 23.03.2017 23:58, Henri Verbeet wrote: >> diff --git a/dlls/winex11.drv/init.c b/dlls/winex11.drv/init.c >> index af2fd92..7efd603 100644 >> --- a/dlls/winex11.drv/init.c >> +++ b/dlls/winex11.drv/init.c >> @@ -375,9 +375,6 @@ static INT X11DRV_ExtEscape( PHYSDEV dev, INT escape, INT in_count, LPCVOID in_d >> RECT rect = physDev->dc_rect; >> >> OffsetRect( &rect, -physDev->dc_rect.left, -physDev->dc_rect.top ); >> - /* The GL drawable may be lagged behind if we don't flush first, so >> - * flush the display make sure we copy up-to-date data */ >> - XFlush( gdi_display ); > > > Shouldn't this be called if GLX_OML_sync_control is not available? It > seems to me that it should either depend on extension availability or be > a separated patch. > It doesn't work (on any current driver), and as far as I'm aware it never did. In that regard it could be a separate commit, yes. I think the change is closely enough related to the other changes and the resulting patch is trivial enough to justify including it, but I don't feel particularly strongly about that.

March 24, 2017

Re: [PATCH v2] winex11: Use GLX_OML_sync_control to synchronise buffer swaps with X11DRV_FLUSH_GL_DRAWABLE requests.

by Jacek Caban

Hi Henri, On 23.03.2017 23:58, Henri Verbeet wrote: > diff --git a/dlls/winex11.drv/init.c b/dlls/winex11.drv/init.c > index af2fd92..7efd603 100644 > --- a/dlls/winex11.drv/init.c > +++ b/dlls/winex11.drv/init.c > @@ -375,9 +375,6 @@ static INT X11DRV_ExtEscape( PHYSDEV dev, INT escape, INT in_count, LPCVOID in_d > RECT rect = physDev->dc_rect; > > OffsetRect( &rect, -physDev->dc_rect.left, -physDev->dc_rect.top ); > - /* The GL drawable may be lagged behind if we don't flush first, so > - * flush the display make sure we copy up-to-date data */ > - XFlush( gdi_display ); Shouldn't this be called if GLX_OML_sync_control is not available? It seems to me that it should either depend on extension availability or be a separated patch. Thanks, Jacek

March 24, 2017

Re: [PATCH] oledb32/tests: Add convert to DBTYPE_NUMERIC tests

by Huw Davies

On Fri, Mar 24, 2017 at 01:11:17AM +0000, Alistair Leslie-Hughes wrote: > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/oledb32/tests/convert.c | 147 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 147 insertions(+) > > diff --git a/dlls/oledb32/tests/convert.c b/dlls/oledb32/tests/convert.c > index b8b3c6f..4e49058 100644 > --- a/dlls/oledb32/tests/convert.c > +++ b/dlls/oledb32/tests/convert.c > @@ -3264,6 +3264,152 @@ static void test_converttoiunknown(void) > ok(dst_len == 44, "got %ld\n", dst_len); > } > > +#define test_numberic_val(dst, array) _test_numberic_val(__LINE__, dst, array); > +static inline void _test_numberic_val(unsigned line, DB_NUMERIC *dst, BYTE *array) > +{ > + int same = !memcmp(dst->val, array, sizeof(dst->val)); > + ok_(__FILE__,line) (same, "Invalid byte array\n"); > + if(!same) > + { > + int i; > + for(i=0; i < sizeof(dst->val); i++) > + ok_(__FILE__,line) (dst->val[i] == array[i], " byte %d got 0x%02x\n", i, dst->val[i]); > + } > +} > + > +static inline void set_numberic_array(BYTE *dst, > + BYTE x0, BYTE x1, BYTE x2, BYTE x3, BYTE x4, BYTE x5, BYTE x6, > + BYTE x7, BYTE x8, BYTE x9, BYTE x10, BYTE x11, BYTE x12, BYTE x13, > + BYTE x14, BYTE x15) > +{ > + dst[0] = x0; dst[1] = x1; dst[2] = x2; dst[3] = x3; > + dst[4] = x4; dst[5] = x5; dst[6] = x6; dst[7] = x7; > + dst[8] = x8; dst[9] = x9; dst[10] = x10; dst[11] = x11; > + dst[12] = x12; dst[13] = x13; dst[14] = x14; dst[15] = x15; > +} > + > +static void test_converttonumeric(void) > +{ > + HRESULT hr; > + DBSTATUS dst_status; > + DBLENGTH dst_len; > + DB_NUMERIC dst; > + BYTE src[20]; > + static WCHAR strW[] = {'1','2','3','.','4','5',0}; > + static WCHAR largeW[] = {'1','2','3','4','5','6','7','8','9','0',0}; > + BSTR bstr; > + FLOAT fvalue = 543.21f; > + VARIANT_BOOL boolean = VARIANT_TRUE; > + LARGE_INTEGER i8; > + BYTE array[16]; > + > + *(INT *)src = 4098; > + dst_len = 0x1234; > + dst.scale = 30; > + memset(dst.val, 0xfe, sizeof(dst.val)); > + hr = IDataConvert_DataConvert(convert, DBTYPE_I4, DBTYPE_NUMERIC, 0, &dst_len, src, &dst, sizeof(dst), 0, &dst_status, 10, 0, 0); > + todo_wine ok(hr == S_OK, "got %08x\n", hr); > + todo_wine ok(dst_status == DBSTATUS_S_OK, "got %08x\n", dst_status); > + todo_wine ok(dst_len == sizeof(dst), "got %ld\n", dst_len); > + todo_wine ok(dst.precision == 10, "returned %d\n", dst.precision); > + todo_wine ok(dst.scale == 0, "returned %d\n", dst.scale); > + todo_wine ok(dst.sign == 1, "returned %d\n", dst.sign); > + set_numberic_array(array, 0x02, 0x10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ); > + todo_wine test_numberic_val(&dst, array); All this seems overly complicated. How about something like: DB_NUMERIC val1 = {10, 0, 1, {0x02, 0x10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }}; /* 4098 */ .... hr = IDataConvert_DataConvert(convert, DBTYPE_I4, DBTYPE_NUMERIC, 0, &dst_len, src, &dst, sizeof(dst), 0, &dst_status, 10, 0, 0); todo_wine ok(hr == S_OK, "got %08x\n", hr); todo_wine ok(dst_status == DBSTATUS_S_OK, "got %08x\n", dst_status); todo_wine ok(dst_len == sizeof(dst), "got %ld\n", dst_len); todo_wine ok(!memcmp(&dst, &val1, sizeof(dst), "mismatch\n"); It doesn't print out the retrieved value on failure, and you could add this if you want. However, once the conversion is actually implemented, you won't need that anyway. You should probably add a test for converting a variant of type VT_NULL too. Huw.

March 24, 2017