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, [e-mail] 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 [e-mail] wrote:
> On 2016-06-05 01:29, [e-mail] 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
> [e-mail]
> 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) [e-mail]
wrote:
> This change sounds wrong to me too.
>
> On 5 June 2016 at 08:26, Thomas Faber [e-mail] wrote:
> > On 2016-06-05 01:29, [e-mail] 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
> > [e-mail]
> > 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) [e-mail]
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) [e-mail]
> wrote:
> > This change sounds wrong to me too.
> >
> > On 5 June 2016 at 08:26, Thomas Faber [e-mail] wrote:
> > > On 2016-06-05 01:29, [e-mail] 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
> > > [e-mail]
> > > http://www.reactos.org/mailman/listinfo/ros-dev