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.
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.
There is another reason for moving it into the container, but that
reason is not public.
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:pharo-dev-boun...@lists.pharo.org] On Behalf Of Thierry
Goubier
Sent: Sunday, April 3, 2016 8:58 PM
To: Pharo Development List <pharo-dev@lists.pharo.org>
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
