Issue 790 in moose-technology: adding Mondrian subview interactively operates on wrong node
Status: New Owner: ---- Labels: Type-Defect Priority-Medium
New issue 790 by google....@ben.coman.com.au: adding Mondrian subview interactively operates on wrong node http://code.google.com/p/moose-technology/issues/detail?id=790
Executing the code snippet below in Mondrian Easel exhibits surprising behaviour. Referring to attached snapshot, clicking on a square incorrectly operates on an ellipse instead. ----8<---- view shape label. view node: 'Ellipses' forIt: [ view shape ellipse size: 30. view interaction whenClickingUpdateNode: [ :value | view forNode: value do: [ view nodes: (1 to: 5). view gridLayout. ] ] withLayoutUpdate: true. view nodes: ( 1 to: 16 ). view gridLayout. ].
view shape label. view node: 'Rectangles' forIt: [ view shape rectangle size: 30. view interaction whenClickingUpdateNode: [ :value | view forNode: value do: [ view nodes: (1 to: 5). view gridLayout. ] ] withLayoutUpdate: true. view nodes: ( 1 to: 16 ). view gridLayout. ].
Attachments: Mondrian add subview interactively.png 10.2 KB
My understanding of Announcements is not so good yet, but here is another opportunity. While I was trying to trace some code execution to troubleshoot a ticket that I logged for Mondrian, I bumped into some behaviour that seems strange, which I would like to understand. At the end I propose a fix.
For the short example, execute [MOEasel open] then <Generate View> on the following: ----8<---- view shape rectangle size: 20. view interaction registerForEvent: MOMouseDown forNode: #yourself updateNode: [ :v | "self halt." view forNode: v do: [ view nodes: (1 to: 2) ] ] updateLayout: true. view nodes: (1 to: 4). ----8<----
Click on a node and observe that node successfully has two nodes within it.
Now uncomment the "self halt" and <Generate View>, then click on a node. When the pre-debugger for 'halt' appears, click <Proceed>. Surprisingly a MNU now occurs in MOAnnouncement>>registerForEvent:forNode:updateNode:updateLayout due to 'ann viewRenderer' now returning nil. Observe this occurs immediately after the execution of 'updateBlock' which contained the 'self halt'
The extended example modifies MOAnnouncement>>registerForEvent:forNode:updateNode:updateLayout as follows... | domainNode btcRenderer1 btcRenderer1test btcRenderer2 btcRenderer2test btcRenderer1test2 | self on: eventClass do: [:ann | "----8<---- begin debug code " btcRenderer1 := ann viewRenderer. btcRenderer1test := (self viewRenderer == ann viewRenderer ). "self halt." btcRenderer2 := ann viewRenderer. btcRenderer2test := (self viewRenderer == ann viewRenderer ). { self viewRenderer. btcRenderer1. btcRenderer2. btcRenderer1test. btcRenderer2test. } inspect. "----8<---- end debug code " ann element removeAllEdges; removeAllNodes.
<Generate View> on the same code as the short example with the 'self halt' commented out, then click on a node. An inspector pops up showing array { aMOViewRenderer. aMOViewRenderer. aMOViewRenderer. true. true. }
Then uncomment the 'self halt' in the extended example, <Generate View> and click on a node. <Proceed> at the pre-debugger. An inspector pops up now showing array { aMOViewRenderer. aMOViewRenderer. nil. true. false. } 'ann viewRenderer ' has lost its value following the 'self halt.'
Proposed fix: It can be seen from the second inspector that 'self viewRenderer' is identical to 'ann viewRenderer' and retains its value across the 'self halt', and so might be used in its place.
thanks for you attention, your thoughts would be appreciated. cheers, -ben
Hi Ben,
Your mail requires a bit more attention span than I can afford right now, and I do not quite understand what the issue actually is. But, this does not mean it went unnoticed :).
Cheers, Doru
On 9 Apr 2012, at 16:09, Ben Coman wrote:
My understanding of Announcements is not so good yet, but here is another opportunity. While I was trying to trace some code execution to troubleshoot a ticket that I logged for Mondrian, I bumped into some behaviour that seems strange, which I would like to understand. At the end I propose a fix.
For the short example, execute [MOEasel open] then <Generate View> on the following: ----8<---- view shape rectangle size: 20. view interaction registerForEvent: MOMouseDown forNode: #yourself updateNode: [ :v | "self halt." view forNode: v do: [ view nodes: (1 to: 2) ] ] updateLayout: true. view nodes: (1 to: 4). ----8<----
Click on a node and observe that node successfully has two nodes within it.
Now uncomment the "self halt" and <Generate View>, then click on a node. When the pre-debugger for 'halt' appears, click <Proceed>. Surprisingly a MNU now occurs in MOAnnouncement>>registerForEvent:forNode:updateNode:updateLayout due to 'ann viewRenderer' now returning nil. Observe this occurs immediately after the execution of 'updateBlock' which contained the 'self halt'
The extended example modifies MOAnnouncement>>registerForEvent:forNode:updateNode:updateLayout as follows... | domainNode btcRenderer1 btcRenderer1test btcRenderer2 btcRenderer2test btcRenderer1test2 | self on: eventClass do: [:ann | "----8<---- begin debug code " btcRenderer1 := ann viewRenderer. btcRenderer1test := (self viewRenderer == ann viewRenderer ). "self halt." btcRenderer2 := ann viewRenderer. btcRenderer2test := (self viewRenderer == ann viewRenderer ). { self viewRenderer. btcRenderer1. btcRenderer2. btcRenderer1test. btcRenderer2test. } inspect. "----8<---- end debug code " ann element removeAllEdges; removeAllNodes.
<Generate View> on the same code as the short example with the 'self halt' commented out, then click on a node. An inspector pops up showing array { aMOViewRenderer. aMOViewRenderer. aMOViewRenderer. true. true. }
Then uncomment the 'self halt' in the extended example, <Generate View> and click on a node. <Proceed> at the pre-debugger. An inspector pops up now showing array { aMOViewRenderer. aMOViewRenderer. nil. true. false. } 'ann viewRenderer ' has lost its value following the 'self halt.'
Proposed fix: It can be seen from the second inspector that 'self viewRenderer' is identical to 'ann viewRenderer' and retains its value across the 'self halt', and so might be used in its place. thanks for you attention, your thoughts would be appreciated. cheers, -ben
-- www.tudorgirba.com
"No matter how many recipes we know, we still value a chef."
'From Pharo1.3 of 16 June 2011 [Latest update: #13315] on 14 April 2012 at 10:21:48 am'!
!MOAnnouncer methodsFor: 'convenient interaction' stamp: 'BenComan 4/14/2012 10:14'! registerForEvent: eventClass forNode: aBlockOrSymbol updateNode: updateBlock updateLayout: aBoolean
| domainNode btcRenderer1 btcRenderer2test btcRenderer3test btcRenderer2 btcRenderer1test | self on: eventClass do: [:ann |
"Execute the following in Mondrian>Easel... view shape rectangle size: 20. view interaction registerForEvent: MOMouseDown forNode: #yourself updateNode: [ :v | view forNode: v do: [ view nodes: (1 to: 2) ] ] updateLayout: true. view nodes: (1 to: 4). ...for case1 when 'self halt' in the debug code below is commented, the 'inspect' shows Array(a MOViewRenderer a MOViewRenderer a MOViewRenderer true true true) ...for case2 when 'self halt' in the debug code below is uncommented, the 'inspect' shows an Array(a MOViewRenderer a MOViewRenderer nil true true false) " "----8<---- debug code begin " btcRenderer1 := ann viewRenderer. btcRenderer1test := ( btcRenderer1 == self viewRenderer ). self halt. "when this line is uncommented for case2, just click <Proceed> at pre-debugger" btcRenderer2 := ann viewRenderer. btcRenderer2test := ( btcRenderer1 == self viewRenderer ). btcRenderer3test := ( btcRenderer1 == btcRenderer2 ). { self viewRenderer. btcRenderer1. btcRenderer2. btcRenderer1test. btcRenderer2test. btcRenderer3test. } inspect. "----8<---- debug code end"
"ann element = the MONode"
ann element removeAllEdges; removeAllNodes.
domainNode := (aBlockOrSymbol moValue: ann element model). ann element resetMetricCachesResursively; resetElementsToLookupUpToTheRoot. ann viewRenderer forNodes: {domainNode} do: updateBlock. aBoolean ifTrue: [ ann viewRenderer root allNodes do: #resetCache. ann viewRenderer root resetElementsToDisplayCacheRecursively. ann element resetCacheInEdges. ann viewRenderer updateWindow. ann viewRenderer root applyLayout] ifFalse: [ ann viewRenderer updateWindow ] ]. ! !
'From Pharo1.3 of 16 June 2011 [Latest update: #13315] on 14 April 2012 at 10:22:08 am'!
!MOAnnouncer methodsFor: 'accessing' stamp: 'BenComan 4/14/2012 10:22'! viewRenderer: anObject "self halt." "to monitor debug case 2, only uncomment this after the mondrian view has been generated but before clicking on a node - not that this halt is not encountered before the one in MOAnnouncer>>registerForEvent:forNode:updateNode:updateLayout:" viewRenderer := anObject! !
Comment #1 on issue 790 by google....@ben.coman.com.au: adding Mondrian subview interactively operates on wrong node http://code.google.com/p/moose-technology/issues/detail?id=790
Uploaded better image.
Attachments: Mondrian add subview interactively.png 12.0 KB
Comment #2 on issue 790 by google....@ben.coman.com.au: adding Mondrian subview interactively operates on wrong node http://code.google.com/p/moose-technology/issues/detail?id=790
This is a bit presumptuous without knowing Mondrian better architecturally, but I can't help thinking that some of the methods named 'forNode' would be more appropriately named 'forNodeValue' - particularly for interaction such as registerForEvent:forNode:updateNode:updateLayout:. Having tried tracing this down further, the root cause is that the "node that is clicked" is not operated on directly. Instead the "value" of the clicked node searched from root. Where the same value occurs multiple times in a graph, a different node might be found for "that value" to apply the operation to rather than the "node that was clicked".
Instead I expect to be able to apply operations to the actual node that I selected. Here is another example... ----8<----- view nodes: (1 to: 5) forEach: [:each | view interaction strongHighlightWhenOver: [ :v | v ]. view shape rectangle withText; size: 20. view nodes: (1 to: each). view gridLayout ]. ----8<----- Executing this in Mondrian Easel (taking for example the third group) small squares 1 & 2 do not highlight when hovering over them, and hovering over small square 3 highlights the large square rather than its small square.
I can get what I consider the usually expected behaviour by modifying MOAnnouncer>>registerForEvent:forNodes:updateShape:updateLayout: replacing... domainNodes := (aBlockOrSymbol moValue: ann element model). nodes := ann viewRenderer nodeForDomainValues: domainNodes. by... nodes := OrderedCollection new add: ann element ; yourself.
I can see where you might want all graph nodes having the same value to highlight when any one is selected, for which a method registerForEvent:forNodeValues: might be appropriate.
It is however not clear to me whether the 'node' in the forNodes is referring to a node in the model or a node in the display. I am presuming the latter - particularly in relation to interactions.
Updates: Status: WontFix
Comment #3 on issue 790 by alexandr...@gmail.com: adding Mondrian subview interactively operates on wrong node http://code.google.com/p/moose-technology/issues/detail?id=790
Most of the mechanisms to update nodes in Mondrian are a kind of hack. As soon as you have two or more nodes with the same model, then the interaction will simply not work.
Regarding your suggestion for for the class MOAnnouncer, it does not have the same effect. It turns some tests yellow.
I turn this issue WontFix because it simply cannot be fixed. Roassal provides a simpler model for building interactive visualization, especially with node update.
moose-technology@googlecode.com wrote:
Updates: Status: WontFix
Comment #3 on issue 790 by alexandr...@gmail.com: adding Mondrian subview interactively operates on wrong node http://code.google.com/p/moose-technology/issues/detail?id=790
Most of the mechanisms to update nodes in Mondrian are a kind of hack. As soon as you have two or more nodes with the same model, then the interaction will simply not work.
Regarding your suggestion for for the class MOAnnouncer, it does not have the same effect. It turns some tests yellow.
I turn this issue WontFix because it simply cannot be fixed. Roassal provides a simpler model for building interactive visualization, especially with node update.
Moose-dev mailing list Moose-dev@iam.unibe.ch https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Fair enough. Roassal was going to be my next question, so I'm glad you are looking at that.
-
Ben Coman -
moose-technology＠googlecode.com -
Tudor Girba