[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
|