[PATCH] comctl32/taskdialog: Implement buttons.

Nikolay Sivov bunglehead at gmail.com
Fri Mar 24 04:44:48 CDT 2017


On 23.03.2017 21:22, Fabian Maurer wrote:
> Signed-off-by: Fabian Maurer <dark.shadow4 at web.de>
> ---
>  dlls/comctl32/taskdialog.c | 224 ++++++++++++++++++++++++++++++++-------------
>  1 file changed, 161 insertions(+), 63 deletions(-)
> 
> diff --git a/dlls/comctl32/taskdialog.c b/dlls/comctl32/taskdialog.c
> index 6b8bcfcae6..037b85e082 100644
> --- a/dlls/comctl32/taskdialog.c
> +++ b/dlls/comctl32/taskdialog.c
> @@ -41,10 +41,13 @@ WINE_DEFAULT_DEBUG_CHANNEL(taskdialog);
>  #define ALIGN_LENGTH(_Len, _Align) _Len = ALIGNED_LENGTH(_Len, _Align)
>  #define ALIGN_POINTER(_Ptr, _Align) _Ptr = ALIGNED_POINTER(_Ptr, _Align)
>  
> -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?

> +static const UINT DIALOG_SPACING_BUTTONS_LEFT = 30; /* minimum distance from the left dialog border */
> +static const UINT DIALOG_SPACING_BUTTON_H = 1; /* Distance between buttons */
> +static const UINT DIALOG_SPACING_BUTTON_W = 4; /* Distance between buttons */
>  
>  static const UINT ID_MAIN_INSTRUCTION = 0xf000;
>  
> @@ -67,6 +70,15 @@ struct taskdialog_template_desc
>      HFONT font;
>  };
>  
> +typedef struct
> +{
> +    int id;
> +    const WCHAR *text;
> +    UINT width;
> +    UINT x;
> +    UINT y;
> +} button_info;
> +
>  static void pixels_to_dialogunits(const struct taskdialog_template_desc *desc, LONG *width, LONG *height)
>  {
>      if (width)
> @@ -83,6 +95,44 @@ static void dialogunits_to_pixels(const struct taskdialog_template_desc *desc, L
>          *height = MulDiv(*height, desc->y_baseunit, 8);
>  }
>  
> +/* used to calculate size for the controls */
> +static RECT text_get_rect(const struct taskdialog_template_desc *desc, const WCHAR *text)
> +{
> +    const WCHAR *textW = NULL;
> +    unsigned int length;
> +    HFONT oldfont;
> +    HDC hdc;
> +    static const WCHAR nulW;
> +    RECT rect = { 0, 0, desc->dialog_width - DIALOG_SPACING * 2, 0}; /* padding left and right of the control */
> +
> +    if (IS_INTRESOURCE(text))
> +    {
> +        if (!(length = LoadStringW(desc->taskconfig->hInstance, (UINT_PTR)text, (WCHAR *)&textW, 0)))
> +        {
> +            WARN("Failed to load text\n");
> +            textW = &nulW;
> +            length = 0;
> +        }
> +    }
> +    else
> +    {
> +        textW = text;
> +        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);
> +
> +    return rect;
> +}
> +

Did you test that buttons are supposed to support multiple lines of text?

>  static void template_write_data(char **ptr, const void *src, unsigned int size)
>  {
>      memcpy(*ptr, src, size);
> @@ -90,7 +140,7 @@ static void template_write_data(char **ptr, const void *src, unsigned int size)
>  }
>  
>  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.

>      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.

> +
> +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.

> +
> +     /* Now that we got them all positioned, create all buttons */
> +    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.

> +
> +    /* Add height for last row and spacing */
> +    desc->dialog_height +=  DIALOG_BUTTON_HEIGHT + DIALOG_SPACING;
> +
> +    HeapFree(GetProcessHeap(), 0, buttons);
>      return size;
>  }
>  
> @@ -295,7 +393,7 @@ static DLGTEMPLATE *create_taskdialog_template(const TASKDIALOGCONFIG *taskconfi
>      desc.dialog_width = min(desc.dialog_width, screen_width);
>  
>      size += taskdialog_add_main_instruction(&desc);
> -    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.

>  
>      template = Alloc(size);
>      if (!template)
> 




More information about the wine-devel mailing list