Re: [ros-dev] [ros-diffs] [hbelusca] 71524: [SHELL32] - Minor stylistic whitespace changes. - Use 'IDA_DESKBROWSER' instead of hardcoding its value. - The shell progman window uses the COLOR_DESKTOP as its hbrBackground: pa...

[Thomas Faber]

On 2016-06-05 01:29, hbelusca(a)svn.reactos.org wrote:
>   -    rcDesk.left =
> GetSystemMetrics(SM_XVIRTUALSCREEN);
>  -    rcDesk.top = GetSystemMetrics(SM_YVIRTUALSCREEN);
>  -    rcDesk.right = rcDesk.left + GetSystemMetrics(SM_CXVIRTUALSCREEN);
>  -    rcDesk.bottom = rcDesk.top + GetSystemMetrics(SM_CYVIRTUALSCREEN);
>  +    rcDesk.left   = GetSystemMetrics(SM_XVIRTUALSCREEN);
>  +    rcDesk.top    = GetSystemMetrics(SM_YVIRTUALSCREEN);
>  +    rcDesk.right  = GetSystemMetrics(SM_CXVIRTUALSCREEN);
>  +    rcDesk.bottom = GetSystemMetrics(SM_CYVIRTUALSCREEN); 
Are you sure about this? "CX" normally means width, so right = left + cx
seems more logical. And SM_XVIRUTALSCREEN is probably 0 most of the
time so it would be hard to notice.

[David Quintana (gigaherz)]

This change sounds wrong to me too.

On 5 June 2016 at 08:26, Thomas Faber <thomas.faber(a)reactos.org> wrote:

>   On 2016-06-05 01:29, hbelusca(a)svn.reactos.org wrote:
> >   -    rcDesk.left =
> > GetSystemMetrics(SM_XVIRTUALSCREEN);
> >  -    rcDesk.top = GetSystemMetrics(SM_YVIRTUALSCREEN);
> >  -    rcDesk.right = rcDesk.left + GetSystemMetrics(SM_CXVIRTUALSCREEN);
> >  -    rcDesk.bottom = rcDesk.top + GetSystemMetrics(SM_CYVIRTUALSCREEN);
> >  +    rcDesk.left   = GetSystemMetrics(SM_XVIRTUALSCREEN);
> >  +    rcDesk.top    = GetSystemMetrics(SM_YVIRTUALSCREEN);
> >  +    rcDesk.right  = GetSystemMetrics(SM_CXVIRTUALSCREEN);
> >  +    rcDesk.bottom = GetSystemMetrics(SM_CYVIRTUALSCREEN); 
>  Are you sure about this? "CX" normally means width, so right = left + cx
>  seems more logical. And SM_XVIRUTALSCREEN is probably 0 most of the
>  time so it would be hard to notice.
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev

[David Quintana (gigaherz)]

Ah I see: it's misusing the rect.

It later passes right+bottom as "cx" and "cy" to CreateWindowEx. I
propose
getting rid of this rect variable in favor of 4 properly-named variables.

On 5 June 2016 at 10:31, David Quintana (gigaherz) <gigaherz(a)gmail.com>
wrote:

>   This change sounds wrong to me too.
>
>  On 5 June 2016 at 08:26, Thomas Faber <thomas.faber(a)reactos.org> wrote:
>
> >   On 2016-06-05 01:29, hbelusca(a)svn.reactos.org
> > wrote:
> > >   -    rcDesk.left =
> > > GetSystemMetrics(SM_XVIRTUALSCREEN);
> > >  -    rcDesk.top = GetSystemMetrics(SM_YVIRTUALSCREEN);
> > >  -    rcDesk.right = rcDesk.left + GetSystemMetrics(SM_CXVIRTUALSCREEN);
> > >  -    rcDesk.bottom = rcDesk.top + GetSystemMetrics(SM_CYVIRTUALSCREEN);
> > >  +    rcDesk.left   = GetSystemMetrics(SM_XVIRTUALSCREEN);
> > >  +    rcDesk.top    = GetSystemMetrics(SM_YVIRTUALSCREEN);
> > >  +    rcDesk.right  = GetSystemMetrics(SM_CXVIRTUALSCREEN);
> > >  +    rcDesk.bottom = GetSystemMetrics(SM_CYVIRTUALSCREEN); 
> >  Are you sure about this? "CX" normally means width, so right = left + cx
> >  seems more logical. And SM_XVIRUTALSCREEN is probably 0 most of the
> >  time so it would be hard to notice.
> >
> >  _______________________________________________
> >  Ros-dev mailing list
> >  Ros-dev(a)reactos.org
> >  http://www.reactos.org/mailman/listinfo/ros-dev 
>
>

[David Quintana (gigaherz)]

Went ahead and fixed it (r71526).

On 5 June 2016 at 10:38, David Quintana (gigaherz) <gigaherz(a)gmail.com>
wrote:

>   Ah I see: it's misusing the rect.
>
>  It later passes right+bottom as "cx" and "cy" to CreateWindowEx. I
> propose
>  getting rid of this rect variable in favor of 4 properly-named variables.
>
>  On 5 June 2016 at 10:31, David Quintana (gigaherz) <gigaherz(a)gmail.com>
>  wrote:
>
> >   This change sounds wrong to me too.
> >
> >  On 5 June 2016 at 08:26, Thomas Faber <thomas.faber(a)reactos.org> wrote:
> >
> > >   On 2016-06-05 01:29, hbelusca(a)svn.reactos.org
> > > wrote:
> > > >   -    rcDesk.left =
> > > > GetSystemMetrics(SM_XVIRTUALSCREEN);
> > > >  -    rcDesk.top = GetSystemMetrics(SM_YVIRTUALSCREEN);
> > > >  -    rcDesk.right = rcDesk.left + GetSystemMetrics(SM_CXVIRTUALSCREEN);
> > > >  -    rcDesk.bottom = rcDesk.top + GetSystemMetrics(SM_CYVIRTUALSCREEN);
> > > >  +    rcDesk.left   = GetSystemMetrics(SM_XVIRTUALSCREEN);
> > > >  +    rcDesk.top    = GetSystemMetrics(SM_YVIRTUALSCREEN);
> > > >  +    rcDesk.right  = GetSystemMetrics(SM_CXVIRTUALSCREEN);
> > > >  +    rcDesk.bottom = GetSystemMetrics(SM_CYVIRTUALSCREEN); 
> > >  Are you sure about this? "CX" normally means width, so right = left + cx
> > >  seems more logical. And SM_XVIRUTALSCREEN is probably 0 most of the
> > >  time so it would be hard to notice.
> > >
> > >  _______________________________________________
> > >  Ros-dev mailing list
> > >  Ros-dev(a)reactos.org
> > >  http://www.reactos.org/mailman/listinfo/ros-dev 
> >
> >  
>