The Inbox: Morphic-ct.1690.mcz

Am 01.10.2020 02:42:35 schrieb [hidden email] <[hidden email]>:

A new version of Morphic was added to project The Inbox:
http://source.squeak.org/inbox/Morphic-ct.1690.mcz

==================== Summary ====================

Name: Morphic-ct.1690
Author: ct
Time: 1 October 2020, 2:42:17.443188 am
UUID: 58a1128e-5547-4236-bc03-0c1f1366bb7b
Ancestors: Morphic-eem.1686

Fixes display invalidation in PluggableListMorph when displayed during updating the list contents.

To avoid recursive #getList sends, since Morphic-mt.1576, the list variable has been cleaned up before sending #getFullList/#getFilteredList. However, in some cases the list morph is redrawn while the #getList operation is being performed, for example if the model code shows a system progress bar (e.g. when loading/refreshing code in Monticello). In the past, this led to drawing artifacts during the list update.

To avoid these artifacts, this version implements the list update as an atomic operation by storing the previous list contents in the variable previousStableList while recomputing the list. In addition, if #getList is curtailed, the previous list contents are restored (which is especially helpful during debugging).

=============== Diff against Morphic-eem.1686 ===============

Item was changed:
ScrollPane subclass: #PluggableListMorph
+ instanceVariableNames: 'list fullList modelToView viewToModel getListSelector getListSizeSelector getListElementSelector getIndexSelector setIndexSelector keystrokeActionSelector autoDeselect lastKeystrokeTime lastKeystrokes lastClickTime doubleClickSelector handlesBasicKeys potentialDropRow hoverRow listMorph keystrokePreviewSelector priorSelection getIconSelector getHelpSelector previousStableList'
- instanceVariableNames: 'list fullList modelToView viewToModel getListSelector getListSizeSelector getListElementSelector getIndexSelector setIndexSelector keystrokeActionSelector autoDeselect lastKeystrokeTime lastKeystrokes lastClickTime doubleClickSelector handlesBasicKeys potentialDropRow hoverRow listMorph keystrokePreviewSelector priorSelection getIconSelector getHelpSelector'
classVariableNames: 'ClearFilterAutomatically ClearFilterDelay FilterableLists FlashOnErrors HighlightHoveredRow HighlightPreSelection MenuRequestUpdatesSelection'
poolDictionaries: ''
category: 'Morphic-Pluggable Widgets'!

!PluggableListMorph commentStamp: 'mt 10/12/2019 11:04' prior: 0!
I am a list widget that uses the change/update mechanism to display model data as a vertical arrangement of (maybe formatted) strings and icons in a scroll pane.

When I am in keyboard focus, type in a letter (or several letters quickly) to go to the next item that begins with that letter. If you enabled the "filterable lists" preference, I will hide all items that do not match the filter.

Special keys (arrow up/down, page up/down, home, end) are also supported.!

Item was changed:
----- Method: PluggableListMorph>>getList (in category 'model access - cached') -----
getList
+ "Answer the (maybe filtered) list to be displayed. Cached result, see #updateList. If the list needs to be updated, do this atomically."
+
+ list = #locked ifTrue: [
+ "The list is already being updated at the moment but needs to be accessed again during the update. To avoid recursion, the update is implemented as an atomic operation. This can happen, for example, when the model fetches data that opens the system progress bar which then will redraw periodically."
+ ^ previousStableList ifNil: #()].
- "Answer the (maybe filtered) list to be displayed. Cached result, see #updateList."
-
^ list ifNil: [
+ list := #locked.
- list := #(). "To make this call safe when re-entering it while fetching the list. This can happen, for example, when the model fetches data that opens the system progress bar which then will redraw periodically."
list := self filterableList
ifTrue: [self getFilteredList]
ifFalse: [self getFullList].
self updateListMorph.
list]!

Item was changed:
----- Method: PluggableListMorph>>updateListFilter (in category 'updating') -----
updateListFilter

| selection |
selection := self selectionIndex = 0 "Avoid fetching #getList here."
ifTrue: [nil]
ifFalse: [self selection].

+ previousStableList := list.
+ [list := nil.
- list := nil.
modelToView := Dictionary new.
viewToModel := Dictionary new.

+ self getList] ensure: [
+ list ifNil: [list := previousStableList].
+ previousStableList := nil].
- self getList.

"Try to restore the last selection."
selection ifNotNil: [self selection: selection].!

Hi Marcel,

didn't you handle the same edge case when you edited this method the latest time? I think the screenshot shared in my previous post is a really ugly think, and depending on your image speed, you have to look at it for a quite long time. I do not want to see this again in my image ...

However, if you know any more elegant way to fix the problem, I'll be glad to apply your suggestions. I think the changes are quite small because the list variable is a private detail and only accessed in two methods in total.

Best,

Christoph

Am 16.06.2021 16:20:40 schrieb [hidden email] <[hidden email]>:

Hi all, hi Marcel,

I just stumbled into another similar incident. I have a complex model that does a lazy-cached computation when it is asked the first time for the icons in a list. When the computation is run, the model spawns a progress bar to communicate the operation to the user. In theory, this would work fine, but in practice, if the computation takes more than 1 second to run, the progress bar will redraw the world and thus an almost-infinite loop occurs. Actually, after ~10 iterations the computation succeeds for an unknown reason, but it definitively is very inefficient and looks somehow awkward:

["awkward-progressbar.png"]

Do we need a second recursive lock somewhere around LazyListMorph >> #icon:? Or would you instead suggest to move this responsibility into the model? Conceptually, however, that approach would feel wrong to me because it is the view which defines the special recursive behavior in this case.

Here is the relevant recursive stack slice:

... stuff in model ...
PluggableMultiColumnListMorph>>iconAt:column:
LazyListMorph>>getListIcon:
LazyListMorph>>icon:
LazyListMorph>>iconExtent
LazyListMorph>>widthToDisplayItem:
LazyListMorph>>item:
LazyListMorph>>drawOn:
LazyListMorph(Morph)>>fullDrawOn:
[] in [] in PluggableMultiColumnListMorph(Morph)>>drawSubmorphsOn:
[] in PluggableMultiColumnListMorph(Morph)>>drawSubmorphsOn:
PluggableMultiColumnListMorph(Morph)>>drawSubmorphsOn:
PluggableMultiColumnListMorph(Morph)>>fullDrawOn:
...
PasteUpMorph>>displayWorld
[] in SystemProgressMorph>>position:label:min:max:
...
... stuff in model ...
PluggableMultiColumnListMorph>>iconAt:column:
LazyListMorph>>getListIcon:
LazyListMorph>>icon:
...

Best,
Christoph