Community
Participate
Working Groups
ScrollableSelectionFeedbackPolicy does not update feedback in case the host figure is moved. This can be reproduced within logic example editor, when moving a selected circuit figure, where nested figures have (clipped) connections to the environment of the circuit figure.
The update problem can be reproduced, as reported by Syed Atif as part of bug #195527, as follows : 1. Add a Circuit to the diagram 2. Add a LED inside the Circuit 3. Add a LED to the diagram surface (and not isnide the Circuit) 4. Resize the Circuit so that the contained LED is not visible. 5. Select the Circuit. 6. Move the Circuit around. Result: Notice the Connection does not move with the Circuit, rather it redraws at the last position where the Circuit was. Note that the issue is only reproducable when the above sketched scenario is performed completely, i.e. the circuit and all other figures are newly created. If the editor is re-opened on an already existing diagram, this cannot be reproduced.
Created attachment 168025 [details] Patch to fix problem. The problem is that the feedback policy receives the event of the host figure being moved before the connection have revalidated their bounds. As we are post M7 I have attached a patch to fix this issue. Anthony, could you please review?
getLayer(LayerConstants.CONNECTION_LAYER).invalidateTree(); is going to repaint the connections later? Alex, can you review the patch too? Syed, you reported this in Bug 195527
(In reply to comment #3) > getLayer(LayerConstants.CONNECTION_LAYER).invalidateTree(); > is going to repaint the connections later? Alex, can you review the patch too? > > Syed, you reported this in Bug 195527 Yes. Will it not be better, performance-wise, if we could refrain from doing an invalidateTree on an entire layer?
The alternative would be to compute the incoming/outgoing connections that would have to be revalidated, which IMHO is not possible in all cases (if the ConnectionAnchors are not navigable, e.g. in case of XYAnchors) and which may probably also not be faster.
Created attachment 168802 [details] alternative patch I have a perfect solution for you I think. Connection has a figure move listeners on the source and target, hence if connection gets the figure move event before this edit policy does you'll be ok, because connection would be invalid when you get to updateFeedback() method and validate connection layer. I suggest that we add figure and layout listener when we start showing selection and remove when we hide selection instead of edit policy activation/deactivation. It would be a performance as well. I attached the patch. Works fine for scenario described above. Anyway, I agree we should avoid invalidating all children of the connection layer. Let me know what you think.
adding figure move and layout listeners when we show selection should ensure that figure move listreners from the connections would receive the move event before the editpolicy does and hence the editpolicy would have all appropriate connections invalidated already. Will be a performance enhancement by itself, because the edit policy would listen to figure move events only when it needs too, i.e. the editpart is selected. Feel free to correct me and enhance the patch further if it seems acceptable.
Good idea! The patch looks good, just one comment: as super.deactivate() calls hideSelection() (via setSelectedState), I think the de-registration of the listeners within deactivate() is now superfluous and could be removed. I would also propose to change the comment within activate() and deactivate() to mention, that figure listeners will be registered/deregistered upon showSelection()/hideSelection().
Do you think we should move this to RC2 so I can push the RC1 build?
Created attachment 168810 [details] Patch incorporating proposed changes to alternative patch. I have incorporated my proposed changes into this patch. Alex, if you are fine with these changes, from my point of view it could be committed, resolving this as part of RC1.
Yes, looks good. Of course I had to delete the old code removing the listeners in deactivate. Just forgot :-)
Anthony, if there aren't any objections from your side, I would propose to now commit this as part of RC1.
(In reply to comment #12) > Anthony, if there aren't any objections from your side, I would propose to now > commit this as part of RC1. OK, commit and then I will run RC1
Changes committed to cvs HEAD. Verified with logic editor example that this issue is resolved.
