On 23.03.2017 21:22, Fabian Maurer wrote:
> Signed-off-by: Fabian Maurer <dark.shadow4(a)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)
>
