The Inbox: Morphic-dtl.634.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

The Inbox: Morphic-dtl.634.mcz

commits-2
A new version of Morphic was added to project The Inbox:
http://source.squeak.org/inbox/Morphic-dtl.634.mcz

==================== Summary ====================

Name: Morphic-dtl.634
Author: dtl
Time: 13 January 2013, 1:48:33.942 pm
UUID: 02716c40-7437-458c-80ff-22679fcd3c04
Ancestors: Morphic-dtl.633

Fix for Morphic layout bug reported by Bob Arning in http://lists.squeakfoundation.org/pipermail/squeak-dev/2013-January/167757.html

Invalidate changed regions in Morph>>layoutBounds: in the same manner as in Morph>>doLayoutIn:

=============== Diff against Morphic-dtl.633 ===============

Item was changed:
  ----- Method: Morph>>layoutBounds: (in category 'layout') -----
  layoutBounds: aRectangle
  "Set the bounds for laying out children of the receiver.
  Note: written so that #layoutBounds can be changed without touching this method"
+ | priorBounds outer inner box |
+ priorBounds := self outerBounds.
- | outer inner |
  outer := self bounds.
  inner := self layoutBounds.
  bounds := aRectangle origin + (outer origin - inner origin) corner:
+ aRectangle corner + (outer corner - inner corner).
+ box := self outerBounds.
+ box = priorBounds
+ ifFalse: [self invalidRect: (priorBounds quickMerge: box)]!
- aRectangle corner + (outer corner - inner corner).!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-dtl.634.mcz

David T. Lewis
This is a fix for the "very old buglet" reported by Bob Arning. I'd appreciate
if someone could review this before I move it to trunk, as it may have performance
implications.

Dave

On Sun, Jan 13, 2013 at 06:49:13PM +0000, [hidden email] wrote:

> A new version of Morphic was added to project The Inbox:
> http://source.squeak.org/inbox/Morphic-dtl.634.mcz
>
> ==================== Summary ====================
>
> Name: Morphic-dtl.634
> Author: dtl
> Time: 13 January 2013, 1:48:33.942 pm
> UUID: 02716c40-7437-458c-80ff-22679fcd3c04
> Ancestors: Morphic-dtl.633
>
> Fix for Morphic layout bug reported by Bob Arning in http://lists.squeakfoundation.org/pipermail/squeak-dev/2013-January/167757.html
>
> Invalidate changed regions in Morph>>layoutBounds: in the same manner as in Morph>>doLayoutIn:
>
> =============== Diff against Morphic-dtl.633 ===============
>
> Item was changed:
>   ----- Method: Morph>>layoutBounds: (in category 'layout') -----
>   layoutBounds: aRectangle
>   "Set the bounds for laying out children of the receiver.
>   Note: written so that #layoutBounds can be changed without touching this method"
> + | priorBounds outer inner box |
> + priorBounds := self outerBounds.
> - | outer inner |
>   outer := self bounds.
>   inner := self layoutBounds.
>   bounds := aRectangle origin + (outer origin - inner origin) corner:
> + aRectangle corner + (outer corner - inner corner).
> + box := self outerBounds.
> + box = priorBounds
> + ifFalse: [self invalidRect: (priorBounds quickMerge: box)]!
> - aRectangle corner + (outer corner - inner corner).!
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-dtl.634.mcz

Chris Muller-3
Thanks Dave, I'll try it out with Maui.  Would the reason there might
be potential performance issues be due to the call to outerBounds or
the fact that it is more liberal with invalidating regions and
therefore causing more drawing code to run?

On Sun, Jan 13, 2013 at 12:53 PM, David T. Lewis <[hidden email]> wrote:

> This is a fix for the "very old buglet" reported by Bob Arning. I'd appreciate
> if someone could review this before I move it to trunk, as it may have performance
> implications.
>
> Dave
>
> On Sun, Jan 13, 2013 at 06:49:13PM +0000, [hidden email] wrote:
>> A new version of Morphic was added to project The Inbox:
>> http://source.squeak.org/inbox/Morphic-dtl.634.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Morphic-dtl.634
>> Author: dtl
>> Time: 13 January 2013, 1:48:33.942 pm
>> UUID: 02716c40-7437-458c-80ff-22679fcd3c04
>> Ancestors: Morphic-dtl.633
>>
>> Fix for Morphic layout bug reported by Bob Arning in http://lists.squeakfoundation.org/pipermail/squeak-dev/2013-January/167757.html
>>
>> Invalidate changed regions in Morph>>layoutBounds: in the same manner as in Morph>>doLayoutIn:
>>
>> =============== Diff against Morphic-dtl.633 ===============
>>
>> Item was changed:
>>   ----- Method: Morph>>layoutBounds: (in category 'layout') -----
>>   layoutBounds: aRectangle
>>       "Set the bounds for laying out children of the receiver.
>>       Note: written so that #layoutBounds can be changed without touching this method"
>> +     | priorBounds outer inner box |
>> +     priorBounds := self outerBounds.
>> -     | outer inner |
>>       outer := self bounds.
>>       inner := self layoutBounds.
>>       bounds := aRectangle origin + (outer origin - inner origin) corner:
>> +                             aRectangle corner + (outer corner - inner corner).
>> +     box := self outerBounds.
>> +     box = priorBounds
>> +             ifFalse: [self invalidRect: (priorBounds quickMerge: box)]!
>> -                             aRectangle corner + (outer corner - inner corner).!
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Morphic-dtl.634.mcz

David T. Lewis
On Sun, Jan 13, 2013 at 07:53:07PM -0600, Chris Muller wrote:
> Thanks Dave, I'll try it out with Maui.  Would the reason there might
> be potential performance issues be due to the call to outerBounds or
> the fact that it is more liberal with invalidating regions and
> therefore causing more drawing code to run?

Yes, possibly so. But really I just want a second set of eyes on it because
it is a seemingly trivial change to a part of the system that I do not know
well, so there is a risk of side effects that I might not understand. I do
not know how to measure the performance impact (if any) of this change, and
I have learned that assuming "this would not hurt anything" can sometimes
be a bad idea.

Thanks for looking at it.

Dave

>
> On Sun, Jan 13, 2013 at 12:53 PM, David T. Lewis <[hidden email]> wrote:
> > This is a fix for the "very old buglet" reported by Bob Arning. I'd appreciate
> > if someone could review this before I move it to trunk, as it may have performance
> > implications.
> >
> > Dave
> >
> > On Sun, Jan 13, 2013 at 06:49:13PM +0000, [hidden email] wrote:
> >> A new version of Morphic was added to project The Inbox:
> >> http://source.squeak.org/inbox/Morphic-dtl.634.mcz
> >>
> >> ==================== Summary ====================
> >>
> >> Name: Morphic-dtl.634
> >> Author: dtl
> >> Time: 13 January 2013, 1:48:33.942 pm
> >> UUID: 02716c40-7437-458c-80ff-22679fcd3c04
> >> Ancestors: Morphic-dtl.633
> >>
> >> Fix for Morphic layout bug reported by Bob Arning in http://lists.squeakfoundation.org/pipermail/squeak-dev/2013-January/167757.html
> >>
> >> Invalidate changed regions in Morph>>layoutBounds: in the same manner as in Morph>>doLayoutIn:
> >>
> >> =============== Diff against Morphic-dtl.633 ===============
> >>
> >> Item was changed:
> >>   ----- Method: Morph>>layoutBounds: (in category 'layout') -----
> >>   layoutBounds: aRectangle
> >>       "Set the bounds for laying out children of the receiver.
> >>       Note: written so that #layoutBounds can be changed without touching this method"
> >> +     | priorBounds outer inner box |
> >> +     priorBounds := self outerBounds.
> >> -     | outer inner |
> >>       outer := self bounds.
> >>       inner := self layoutBounds.
> >>       bounds := aRectangle origin + (outer origin - inner origin) corner:
> >> +                             aRectangle corner + (outer corner - inner corner).
> >> +     box := self outerBounds.
> >> +     box = priorBounds
> >> +             ifFalse: [self invalidRect: (priorBounds quickMerge: box)]!
> >> -                             aRectangle corner + (outer corner - inner corner).!
> >>
> >