[squeak-dev] The Inbox: Graphics-kfr.434.mcz

Levente Uzonyi leves at caesar.elte.hu
Sun Jul 12 14:10:19 UTC 2020


Hi Karl,

On Sun, 12 Jul 2020, karl ramberg wrote:

> Hi,
> 
> In Rectangle>>merging: it is quite a bit slower to use points topLeft and bottomRight, than reading out the min and max of each side.

Indeed. Each iteration creates a new point if you use points.

> Also looping with allButFirstDo: is slower than the original method.

I suppose your list of rectangles was an OrderedCollection when you 
measured performance because for most SequenceableCollections 
#allButFirstDo: is slightly faster than #do: by definition.
OrderedCollection overrides #do: for better performance but does not 
have such implementation for #allButFirstDo:/#allButLastDo:.


Levente

> 
> This seems to be fastest:
> 
> Rectangle>>merging: listOfRects
> "A number of callers of merge: should use this method."
> | minX minY maxX maxY |
> minX := listOfRects first topLeft x.
> minY := listOfRects first topLeft y.
> maxX := listOfRects first bottomRight x.
> maxY := listOfRects first bottomRight y.
> listOfRects do: [:r | minX := minX min: r topLeft x.
>              minY := minY min: r topLeft y.
>              maxX := maxX max: r bottomRight x.
>              maxY := maxY max: r bottomRight y].
> ^self origin: minX at minY corner: maxX at maxY
> 
> Best,
> Karl
> 
> On Sat, Jul 11, 2020 at 3:49 PM Levente Uzonyi <leves at caesar.elte.hu> wrote:
>       Hi Karl,
>
>       While both changes look right, they modify the behavior of those methods
>       when their argument is an empty list.
>       Instead of raising an error, they'll return a Rectangle with both fields
>       set to nil.
>       If the argument is really a list - aka SequenceableCollection, I suggest
>       using more specific methods instead of #do: to process the list in order
>       to trigger the error as soon as possible.
>       For example, use #first to get the first element and #allButFirstDo: to
>       iterate over the rest. That would simplify the method as well by making
>       the nil checks unnecessary.
> 
>
>       Levente
>
>       On Sat, 11 Jul 2020, commits at source.squeak.org wrote:
>
>       > A new version of Graphics was added to project The Inbox:
>       > http://source.squeak.org/inbox/Graphics-kfr.434.mcz
>       >
>       > ==================== Summary ====================
>       >
>       > Name: Graphics-kfr.434
>       > Author: kfr
>       > Time: 11 July 2020, 10:18:06.272175 am
>       > UUID: 0e0125f6-3040-6247-9e5a-43bfd1896f84
>       > Ancestors: Graphics-mt.433
>       >
>       > I wanted to subclass Rectangle with a instance variable to carry some state, but these methodes indirected to Point>>corner: so it broke the override and returned a ordinary Rectangle instead of my fancy new
>       subclass SuperRectangle
>       >
>       > =============== Diff against Graphics-mt.433 ===============
>       >
>       > Item was changed:
>       >  ----- Method: Rectangle class>>encompassing: (in category 'instance creation') -----
>       >  encompassing: listOfPoints
>       >       "A number of callers of encompass: should use this method."
>       >       | topLeft bottomRight |
>       >       topLeft := bottomRight := nil.
>       >       listOfPoints do:
>       >               [:p | topLeft == nil
>       >                       ifTrue: [topLeft := bottomRight := p]
>       >                       ifFalse: [topLeft := topLeft min: p.
>       >                                       bottomRight := bottomRight max: p]].
>       > +     ^self origin: topLeft corner: bottomRight!
>       > -     ^ topLeft corner: bottomRight!
>       >
>       > Item was changed:
>       >  ----- Method: Rectangle class>>merging: (in category 'instance creation') -----
>       >  merging: listOfRects
>       >       "A number of callers of merge: should use this method."
>       >       | minX minY maxX maxY |
>       >       listOfRects
>       >               do: [:r | minX
>       >                               ifNil: [minX := r topLeft x. minY := r topLeft y.
>       >                                       maxX := r bottomRight x. maxY := r bottomRight y]
>       >                               ifNotNil: [minX := minX min: r topLeft x. minY := minY min: r topLeft y.
>       >                                       maxX := maxX max: r bottomRight x. maxY := maxY max: r bottomRight y]].
>       > +     ^ self origin:minX at minY corner: maxX at maxY!
>       > -     ^ minX at minY corner: maxX at maxY!
> 
> 
>


More information about the Squeak-dev mailing list