Re: [PATCH v3 0/11] MR557: mciqtz32: Correct video window behavior by creating default window.

[Akihiro Sagawa (＠sgwaki)]

On Tue Aug  2 05:14:16 2022 +0000, **** wrote:
> Zebediah Figura (she/her) replied on the mailing list:
> ```
> On 7/31/22 06:53, Akihiro Sagawa wrote:
> > @@ -1573,6 +1575,37 @@ static void test_video_window(void)
> >           todo_wine_if(i != 3)
> >           ok(style == expected, "hwnd %p: got %#lx, expected %#lx\n",
> > video_window, style, expected);
> >
> > +        /* get source video size */
> > +        err = mciSendCommandW(wDeviceID, MCI_WHERE,
> > MCI_DGV_WHERE_SOURCE, (DWORD_PTR)&parm);
> > +        ok(!err,"mciCommand where source returned %s\n", dbg_mcierr(err));
> > +        src_rc = parm.where.rc;
> > This should just be equal to the whole video rect (0,0)-(64,32) [unless, 
> > I suspect, it is set by MCI_DGV_PUT_SOURCE]; would you mind adding an 
> > explicit test for that?
> > +
> > +        /* test default video destination size */
> > +        err = mciSendCommandW(wDeviceID, MCI_WHERE,
> > MCI_DGV_WHERE_DESTINATION, (DWORD_PTR)&parm);
> > +        ok(!err,"mciCommand where destination returned %s\n", dbg_mcierr(err));
> > +        if (style & (WS_POPUP|WS_CHILD))
> > +            rc = src_rc;
> > +        else
> > +            GetClientRect(video_window, &rc);
> > +        todo_wine_if(style & (WS_POPUP|WS_CHILD))
> > +        ok(EqualRect(&parm.where.rc, &rc), "got %s, expected %s\n",
> > wine_dbgstr_rect(&parm.where.rc), wine_dbgstr_rect(&rc));
> > And this should too. The test here is a bit awkward as such because the 
> > destination and source rect are *always* equal, and separately, the 
> > client rect will also be equal to the video size if the window is a 
> > popup or child.
> > +
> > +        /* test default video window size */
> > +        rc = src_rc;
> > +        AdjustWindowRect(&rc, WS_OVERLAPPEDWINDOW, FALSE); /* sigh */
> > +        OffsetRect(&rc, -rc.left, -rc.top);
> > +        rc.right = max(rc.right, GetSystemMetrics(SM_CXMIN));
> > I gather "sigh" here means "Windows is broken", in the way spelled out 
> > in 8/11, right? It wouldn't hurt to spell that out explicitly ;-)
>
> wine-gitlab mailing list -- wine-gitlab(a)winehq.org
> To unsubscribe send an email to wine-gitlab-leave(a)winehq.org
> ```
> @@ -1573,6 +1575,37 @@ static void test_video_window(void)
>           todo_wine_if(i != 3)
>           ok(style == expected, "hwnd %p: got %#lx, expected %#lx\n", video_window, style, expected);
>   
> +        /* get source video size */
> +        err = mciSendCommandW(wDeviceID, MCI_WHERE, MCI_DGV_WHERE_SOURCE, (DWORD_PTR)&parm);
> +        ok(!err,"mciCommand where source returned %s\n", dbg_mcierr(err));
> +        src_rc = parm.where.rc;
> This should just be equal to the whole video rect (0,0)-(64,32) [unless, 
> I suspect, it is set by MCI_DGV_PUT_SOURCE]; would you mind adding an 
> explicit test for that?

Not at all. Actually, video rect is (0,0)-(32,24).
```
> +
> +        /* test default video destination size */
> +        err = mciSendCommandW(wDeviceID, MCI_WHERE, MCI_DGV_WHERE_DESTINATION, (DWORD_PTR)&parm);
> +        ok(!err,"mciCommand where destination returned %s\n", dbg_mcierr(err));
> +        if (style & (WS_POPUP|WS_CHILD))
> +            rc = src_rc;
> +        else
> +            GetClientRect(video_window, &rc);
> +        todo_wine_if(style & (WS_POPUP|WS_CHILD))
> +        ok(EqualRect(&parm.where.rc, &rc), "got %s, expected %s\n", wine_dbgstr_rect(&parm.where.rc), wine_dbgstr_rect(&rc));
> And this should too. The test here is a bit awkward as such because the 
> destination and source rect are *always* equal, and separately, the 
> client rect will also be equal to the video size if the window is a 
> popup or child.

Minimum width of video window is SM_CXMIN. So, if the video width less than SM_CXMIN approximately, destination rect isn't identical to the source one.

> +
> +        /* test default video window size */
> +        rc = src_rc;
> +        AdjustWindowRect(&rc, WS_OVERLAPPEDWINDOW, FALSE); /* sigh */
> +        OffsetRect(&rc, -rc.left, -rc.top);
> +        rc.right = max(rc.right, GetSystemMetrics(SM_CXMIN));
> I gather "sigh" here means "Windows is broken", in the way spelled out 
> in 8/11, right? It wouldn't hurt to spell that out explicitly ;-)

Right.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/557#note_5586

[PATCH v3 0/11] MR557: mciqtz32: Correct video window behavior by creating default window.

[Zebediah Figura (she/her)]

On 8/3/22 09:49, Akihiro Sagawa (@sgwaki) wrote:
> Not at all. Actually, video rect is (0,0)-(32,24).

Oops, misremembered it :-)
> ```
> > +
> > +        /* test default video destination size */
> > +        err = mciSendCommandW(wDeviceID, MCI_WHERE, MCI_DGV_WHERE_DESTINATION, (DWORD_PTR)&parm);
> > +        ok(!err,"mciCommand where destination returned %s\n", dbg_mcierr(err));
> > +        if (style & (WS_POPUP|WS_CHILD))
> > +            rc = src_rc;
> > +        else
> > +            GetClientRect(video_window, &rc);
> > +        todo_wine_if(style & (WS_POPUP|WS_CHILD))
> > +        ok(EqualRect(&parm.where.rc, &rc), "got %s, expected %s\n", wine_dbgstr_rect(&parm.where.rc), wine_dbgstr_rect(&rc));
> > And this should too. The test here is a bit awkward as such because the
> > destination and source rect are *always* equal, and separately, the
> > client rect will also be equal to the video size if the window is a
> > popup or child.
>
> Minimum width of video window is SM_CXMIN. So, if the video width less than SM_CXMIN approximately, destination rect isn't identical to the source one.

IIRC when I tested, MCI_DGV_WHERE_DESTINATION was always equal to the 
source rect by default, whereas the client rect might be bigger due to 
SM_CXMIN. But I might be misremembering that.