<div dir="ltr"><div>This is what I would do:</div><div><br></div>merging: listOfRects <br>    "A number of callers of merge: should use this method."<br>    ^ listOfRects<br>        inject:<br>            (self<br>                origin: Float infinity @ Float infinity<br>                corner: Float infinity negated @ Float infinity negated)<br>        into: [ : rect : each | rect quickMerge: each ]<br><div><br></div><div>With #quickMerge:, you're only creating a new Rectangle when it needed to grow to accomodate the current element, but many of the elements might fit completely, resulting in no additional instantiation.  Do an analysis of how many temporary Point objects we create -- hint: it's a ton -- and my own attempts to optimize that in my Kml framework were not fruitful (e.g., ~1%), with a trade-off of duplicating implementation across multiple methods and making the system's code harder to read and understand and maintain.</div><div><br></div><div> - Chris</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 17, 2021 at 11:27 AM 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">On Wed, 17 Feb 2021, Marcel Taeumel wrote:<br>
<br>
> Hmm... looking at performance ... why not just add #asSequenceableCollection, which would only impact Set arguments?<br>
<br>
I don't think we have such method. Adding one would raise the usual <br>
question: should it create a new collection or return self if self <br>
already to the requested collection kind?<br>
IIRC currently the only outlier is #asOrderedCollection which always <br>
creates a copy when the receiver is an OrderedCollection, and that <br>
property is being relied on, so it can't be changed...<br>
<br>
> As a programmer, I do not want to choose between #merge: and #quickMerge:. Or similar. :-)<br>
<br>
IMO, we simply need one quick solution. If you have a Set, you may need <br>
that help from the library even more.<br>
#quickMerge: is only good for merging two rectangles. It does not solve <br>
the GC issue #merge: has.<br>
<br>
<br>
Levente<br>
<br>
> <br>
> Best,<br>
> Marcel<br>
><br>
>       Am 13.02.2021 17:05:07 schrieb <a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a> <<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>>:<br>
><br>
>       Nicolas Cellier uploaded a new version of Graphics to project The Inbox:<br>
>       <a href="http://source.squeak.org/inbox/Graphics-nice.446.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/inbox/Graphics-nice.446.mcz</a><br>
><br>
>       ==================== Summary ====================<br>
><br>
>       Name: Graphics-nice.446<br>
>       Author: nice<br>
>       Time: 13 February 2021, 5:04:52.453325 pm<br>
>       UUID: d13c1db2-370f-4fa6-b7ae-e6766bf0c8fb<br>
>       Ancestors: Graphics-dtl.445<br>
><br>
>       Let Rectangle merging:/encompassing: an unordered collection.<br>
><br>
>       =============== Diff against Graphics-dtl.445 ===============<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 := listOfPoints anyOne.<br>
>       + listOfPoints do:<br>
>       - topLeft := bottomRight := listOfPoints first.<br>
>       - listOfPoints allButFirstDo:<br>
>       [:p |topLeft := topLeft min: p.<br>
>       + bottomRight := bottomRight max: p].<br>
>       - bottomRight := bottomRight max: p].<br>
>       ^self origin: topLeft corner: bottomRight<br>
>       !<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>
>       + | aRectangle bottomRight topLeft |<br>
>       + aRectangle := listOfRects anyOne.<br>
>       + topLeft := aRectangle topLeft.<br>
>       + bottomRight := aRectangle bottomRight.<br>
>       - | bottomRight topLeft |<br>
>       - topLeft := listOfRects first topLeft.<br>
>       - bottomRight := listOfRects first bottomRight.<br>
>       listOfRects<br>
>       + do: [:r | topLeft := topLeft min: r topLeft.<br>
>       - allButFirstDo: [:r | topLeft := topLeft min: r topLeft.<br>
>       bottomRight := bottomRight max: r bottomRight].<br>
>       ^self origin: topLeft corner: bottomRight.<br>
>       !<br>
> <br>
> <br>
> <br>
><br>
<br>
</blockquote></div>