<div dir="ltr"><div>Yes, I tested with OrderedCollection. <br></div><div>Changed to Array and the speed was better.<br></div><div>It seems most senders of the Rectangle>>merging: are with Array eg: subBnds := Rectangle merging: (submorphs collect: [:m | m fullBounds]).</div><div>But DamageRecorder is an OrderedCollection for example.</div><div><br></div><div>I'm not sure how big impact speed will have here though. <br></div><div>I can't measure speed difference for small collections with up to 5000 elements (which will probably cover most use cases)</div><div><br></div><div>Change set in attachment. <br></div><div><br></div><div>Best,</div><div>Karl<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Jul 12, 2020 at 4:12 PM Levente Uzonyi <<a href="mailto:leves@caesar.elte.hu">leves@caesar.elte.hu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Karl,<br>
<br>
On Sun, 12 Jul 2020, karl ramberg wrote:<br>
<br>
> Hi,<br>
> <br>
> 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.<br>
<br>
Indeed. Each iteration creates a new point if you use points.<br>
<br>
> Also looping with allButFirstDo: is slower than the original method.<br>
<br>
I suppose your list of rectangles was an OrderedCollection when you <br>
measured performance because for most SequenceableCollections <br>
#allButFirstDo: is slightly faster than #do: by definition.<br>
OrderedCollection overrides #do: for better performance but does not <br>
have such implementation for #allButFirstDo:/#allButLastDo:.<br>
<br>
<br>
Levente<br>
<br>
> <br>
> This seems to be fastest:<br>
> <br>
> Rectangle>>merging: listOfRects<br>
> "A number of callers of merge: should use this method."<br>
> | minX minY maxX maxY |<br>
> minX := listOfRects first topLeft x.<br>
> minY := listOfRects first topLeft y.<br>
> maxX := listOfRects first bottomRight x.<br>
> maxY := listOfRects first bottomRight y.<br>
> listOfRects do: [:r | minX := minX min: r topLeft x.<br>
> minY := minY min: r topLeft y.<br>
> maxX := maxX max: r bottomRight x.<br>
> maxY := maxY max: r bottomRight y].<br>
> ^self origin: minX@minY corner: maxX@maxY<br>
> <br>
> Best,<br>
> Karl<br>
> <br>
> On Sat, Jul 11, 2020 at 3:49 PM Levente Uzonyi <<a href="mailto:leves@caesar.elte.hu" target="_blank">leves@caesar.elte.hu</a>> wrote:<br>
> Hi Karl,<br>
><br>
> While both changes look right, they modify the behavior of those methods<br>
> when their argument is an empty list.<br>
> Instead of raising an error, they'll return a Rectangle with both fields<br>
> set to nil.<br>
> If the argument is really a list - aka SequenceableCollection, I suggest<br>
> using more specific methods instead of #do: to process the list in order<br>
> to trigger the error as soon as possible.<br>
> For example, use #first to get the first element and #allButFirstDo: to<br>
> iterate over the rest. That would simplify the method as well by making<br>
> the nil checks unnecessary.<br>
> <br>
><br>
> Levente<br>
><br>
> On Sat, 11 Jul 2020, <a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a> wrote:<br>
><br>
> > A new version of Graphics was added to project The Inbox:<br>
> > <a href="http://source.squeak.org/inbox/Graphics-kfr.434.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/inbox/Graphics-kfr.434.mcz</a><br>
> ><br>
> > ==================== Summary ====================<br>
> ><br>
> > Name: Graphics-kfr.434<br>
> > Author: kfr<br>
> > Time: 11 July 2020, 10:18:06.272175 am<br>
> > UUID: 0e0125f6-3040-6247-9e5a-43bfd1896f84<br>
> > Ancestors: Graphics-mt.433<br>
> ><br>
> > 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<br>
> subclass SuperRectangle<br>
> ><br>
> > =============== Diff against Graphics-mt.433 ===============<br>
> ><br>
> > Item was changed:<br>
> > ----- Method: Rectangle class>>encompassing: (in category 'instance creation') -----<br>
> > encompassing: listOfPoints<br>
> > "A number of callers of encompass: should use this method."<br>
> > | topLeft bottomRight |<br>
> > topLeft := bottomRight := nil.<br>
> > listOfPoints do:<br>
> > [:p | topLeft == nil<br>
> > ifTrue: [topLeft := bottomRight := p]<br>
> > ifFalse: [topLeft := topLeft min: p.<br>
> > bottomRight := bottomRight max: p]].<br>
> > + ^self origin: topLeft corner: bottomRight!<br>
> > - ^ topLeft corner: bottomRight!<br>
> ><br>
> > Item was changed:<br>
> > ----- Method: Rectangle class>>merging: (in category 'instance creation') -----<br>
> > merging: listOfRects<br>
> > "A number of callers of merge: should use this method."<br>
> > | minX minY maxX maxY |<br>
> > listOfRects<br>
> > do: [:r | minX<br>
> > ifNil: [minX := r topLeft x. minY := r topLeft y.<br>
> > maxX := r bottomRight x. maxY := r bottomRight y]<br>
> > ifNotNil: [minX := minX min: r topLeft x. minY := minY min: r topLeft y.<br>
> > maxX := maxX max: r bottomRight x. maxY := maxY max: r bottomRight y]].<br>
> > + ^ self origin:minX@minY corner: maxX@maxY!<br>
> > - ^ minX@minY corner: maxX@maxY!<br>
> <br>
> <br>
><br>
</blockquote></div>