[squeak-dev] The Inbox: Graphics-kfr.434.mcz
karl ramberg
karlramberg at gmail.com
Sun Jul 12 15:17:06 UTC 2020
Yes, I tested with OrderedCollection.
Changed to Array and the speed was better.
It seems most senders of the Rectangle>>merging: are with Array eg: subBnds
:= Rectangle merging: (submorphs collect: [:m | m fullBounds]).
But DamageRecorder is an OrderedCollection for example.
I'm not sure how big impact speed will have here though.
I can't measure speed difference for small collections with up to 5000
elements (which will probably cover most use cases)
Change set in attachment.
Best,
Karl
On Sun, Jul 12, 2020 at 4:12 PM Levente Uzonyi <leves at caesar.elte.hu> wrote:
> 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!
> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200712/213e18ff/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: RectangleMergeTesting.1.cs
Type: application/octet-stream
Size: 2688 bytes
Desc: not available
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200712/213e18ff/attachment-0001.obj>
More information about the Squeak-dev
mailing list
|