-----Original Message----- From: Pharo-dev [mailto:[email protected]] On Behalf Of Thierry Goubier Sent: Sunday, April 3, 2016 9:52 PM To: [email protected] Subject: Re: [Pharo-dev] [bloc] shape size?
Hi Henrik, Le 03/04/2016 21:32, Henrik Nergaard a écrit : > I changed it mostly because it was messing up the color when you did > mouse-over (required to check the owner color to set it correctly when > there was a selection, the code was ugly and with corner cases). Ok. > Encapsulating a morph with another morph just for a background color > is kind of waste IMO. Causing unnecessary overhead for > rendering/layout/eventHandling etc... Rendering maybe a bit. Layout, maybe a bit. Event handling? Oh well, row morphs contents are already a tree of morphs at least 3 deep, what will be the effect of one more doing nothing? > Mesurable? Don’t think I checked, but in the long run there is less > allocations, less layout etc.. I guess that, given how FT works, it is hidden in the number of Morph creation per drawOn: :) > I would also argue that > ------------------ > (highligtedRowIndexes includes: rowIndex) ifTrue: [ > row selectionColor: (self owner colorForSelection: > primarySelectionIndex = rowIndex) ]. > ----------------- > Is better than > ----------------- > > rowSubviews add: ((highligtedRowIndexes includes: rowIndex) > ifTrue: [ > "IMPORTANT: I need to set owner to nil because > otherwise it will trigger an > invalidation of the owner when adding morph to > selectionMorph, causing an > infinite loop" > self > toSelectionRow: (row privateOwner: nil) > primary: primarySelectionIndex = > rowIndex ] > ifFalse: [ row ]) ]. > ----------------------- <I like the game with #privateOwner: in the FT code. Tells you a lot about ownership in Morphic :) <At the same time, your new code isn't very clear. It is written just as the setting of a theme property where in fact it is setting the row as selected. When pointing it out, I totally agree. Will fix that ;) > Selection could be and drawn by the container, but then again you > would need much more code and special logic (updating the > damageRecorder with correct rectangles when selection changes for > example) than to just extend and specialize one morph to be a row. <I'm doing the selection draw by container, and, honestly, there is none of the complexity you describe. Selection is a model operation anyway, so it usually <triggers a redraw (because selection may move you elsewhere, if the model so chooses); if you have some caching in place (that FT hasn't), you gain a bit upon <redraw, but nothing noticeable. No there won't be that complexity, I was thinking of mouse-over not the selection; apologies. There is another reason for moving it into the container, but that reason is not public. Ah, Yes; even/odd color difference per row. > I would much rather prefer one specialized morph doing its thing, than > encapsulate it. I think the FTTableRowMorph has other duties anyway, so giving him that additional role is fine. Regards, Thierry > Best regards, > Henrik > > -----Original Message----- > From: Pharo-dev [mailto:[email protected]] On Behalf > Of Thierry Goubier > Sent: Sunday, April 3, 2016 8:58 PM > To: Pharo Development List <[email protected]> > Subject: Re: [Pharo-dev] [bloc] shape size? > > Le 03/04/2016 20:00, Henrik Nergaard a écrit : >> >>> Let me return you the question then: do you do a composition of >>> submorphs if you're trying to get a different drawOn:? >> >>> Oh, ok, that's true FastTable does it for the selection... changing >>> background color by encapsulating a row in another Morph with the >>> right background color. >> >> Did, not anymore. >> >> FTTableRowMorph>>#selectionColor: > > Had to check the following code to be sure: > > (highligtedRowIndexes includes: rowIndex) ifTrue: [ > row selectionColor: (self owner colorForSelection: > primarySelectionIndex = rowIndex) ]. > > Was there a measurable inpact when changing that? > > Because I have now three ways of doing this, and all of them have trade-offs. > For example the one above suppose that row items are a specific kind of > Morph, i.e. FTTableRowMorph; one could do without a dedicated Morph subclass > for rows and display the selection as a transparent colored rectangle over > the row; this is what I do. > > Thierry > Best regards, Henrik
