Re: winex11.drv: Use IsRectEmpty() instead of open coding it.

[Alexandre Julliard]

Michael Stefaniuc <mstefani(a)redhat.de> writes:
> @@ -134,7 +134,7 @@ static inline void reset_bounds( RECT *bounds )
>
> static inline void add_bounds_rect( RECT *bounds, const RECT *rect )
>  {
> -    if (rect->left >= rect->right || rect->top >= rect->bottom) return;
> +    if (IsRectEmpty(rect)) return;

I'm not sure it's an improvement to add a function call in such a simple
inline function.

-- 
Alexandre Julliard
julliard(a)winehq.org

winex11.drv: Use IsRectEmpty() instead of open coding it.

[Ken Thomases]

On May 4, 2016, at 2:58 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
> Michael Stefaniuc <mstefani(a)redhat.de> writes:
> > @@ -134,7 +134,7 @@ static inline void reset_bounds( RECT *bounds )
> >
> > static inline void add_bounds_rect( RECT *bounds, const RECT *rect )
> > {
> > -    if (rect->left >= rect->right || rect->top >= rect->bottom) return;
> > +    if (IsRectEmpty(rect)) return;
>
> I'm not sure it's an improvement to add a function call in such a simple
> inline function.

All of these sorts of patches have made me wonder if these RECT-related utility functions shouldn't themselves be defined static inline in the headers (as well as having real versions for export).

-Ken

winex11.drv: Use IsRectEmpty() instead of open coding it.

[Alexandre Julliard]

Ken Thomases <ken(a)codeweavers.com> writes:
> On May 4, 2016, at 2:58 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
> > Michael Stefaniuc <mstefani(a)redhat.de> writes:
> > > @@ -134,7 +134,7 @@ static inline void reset_bounds( RECT *bounds )
> > >
> > > static inline void add_bounds_rect( RECT *bounds, const RECT *rect )
> > > {
> > > -    if (rect->left >= rect->right || rect->top >= rect->bottom) return;
> > > +    if (IsRectEmpty(rect)) return;
> >
> > I'm not sure it's an improvement to add a function call in such a simple
> > inline function.
>
> All of these sorts of patches have made me wonder if these
> RECT-related utility functions shouldn't themselves be defined static
> inline in the headers (as well as having real versions for export).

If we are making heavy use of them, then yes, that would probably be
worth it.

-- 
Alexandre Julliard
julliard(a)winehq.org

winex11.drv: Use IsRectEmpty() instead of open coding it.

[Michael Stefaniuc]

On 05/04/2016 10:15 AM, Alexandre Julliard wrote:
> Ken Thomases <ken(a)codeweavers.com> writes:
> > On May 4, 2016, at 2:58 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
> > > Michael Stefaniuc <mstefani(a)redhat.de> writes:
> > > > @@ -134,7 +134,7 @@ static inline void reset_bounds( RECT *bounds )
> > > >
> > > > static inline void add_bounds_rect( RECT *bounds, const RECT *rect )
> > > > {
> > > > -    if (rect->left >= rect->right || rect->top >= rect->bottom) return;
> > > > +    if (IsRectEmpty(rect)) return;
> > >
> > > I'm not sure it's an improvement to add a function call in such a simple
> > > inline function.
> >
> > All of these sorts of patches have made me wonder if these
> > RECT-related utility functions shouldn't themselves be defined static
> > inline in the headers (as well as having real versions for export).
>
> If we are making heavy use of them, then yes, that would probably be
> worth it.
> Yes, I have thought of that already.
> But the amount of patches and other developers requesting it makes for
> the better "more convincing arguments" ;-P

bye
	michael

winex11.drv: Use IsRectEmpty() instead of open coding it.

[Michael Stefaniuc]

On 05/04/2016 10:28 AM, Michael Stefaniuc wrote:
> On 05/04/2016 10:15 AM, Alexandre Julliard wrote:
> > Ken Thomases <ken(a)codeweavers.com> writes:
> > > On May 4, 2016, at 2:58 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
> > > > Michael Stefaniuc <mstefani(a)redhat.de> writes:
> > > > > @@ -134,7 +134,7 @@ static inline void reset_bounds( RECT *bounds )
> > > > >
> > > > > static inline void add_bounds_rect( RECT *bounds, const RECT *rect )
> > > > > {
> > > > > -    if (rect->left >= rect->right || rect->top >= rect->bottom) return;
> > > > > +    if (IsRectEmpty(rect)) return;
> > > >
> > > > I'm not sure it's an improvement to add a function call in such a simple
> > > > inline function.
> > >
> > > All of these sorts of patches have made me wonder if these
> > > RECT-related utility functions shouldn't themselves be defined static
> > > inline in the headers (as well as having real versions for export).
> >
> > If we are making heavy use of them, then yes, that would probably be
> > worth it.
> > Yes, I have thought of that already.
> > But the amount of patches and other developers requesting it makes for
> > the better "more convincing arguments" ;-P
> > And here are the hard numbers for wine-1.9.9-67-g9eaa372, give or take:
> > wine$ git grep -w SetRect | wc -l
> > 566
> > wine$ git grep -w SetRectEmpty | wc -l
> > 123
> > wine$ git grep -w IsRectEmpty | wc -l
> > 89

I still have some more SetRect() calls to add, should be around 200-300.
SetRectEmpty() I see only 3 more in the generated diff. And
for IsRectEmpty() I have submitted everything.

bye
	michael