Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 461629

Summary:	[CSS] Diagram refresh undo/redo problems
Product:	[Modeling] Papyrus	Reporter:	Christian Damus <give.a.damus>
Component:	Diagram	Assignee:	Christian Damus <give.a.damus>
Status:	RESOLVED FIXED	QA Contact:
Severity:	major
Priority:	P3	CC:	cletavernier, papyrus-bugs
Version:	1.1.0
Target Milestone:	---
Hardware:	All
OS:	All
URL:	http://youtu.be/bv2Gozopha0
See Also:	https://git.eclipse.org/r/43341 https://git.eclipse.org/c/papyrus/org.eclipse.papyrus.git/commit/?id=f584936c1eb00c25c8d082b768a09de8e6fd8622 https://git.eclipse.org/c/papyrus/org.eclipse.papyrus.git/commit/?id=076626232757a6cc017b64d4d9d9d3cbe31906a1
Whiteboard:
Bug Depends on:
Bug Blocks:	433206

Description Christian Damus

2015-03-06 20:17:44 EST

Papyrus Mars M5

I'm not sure how much of this is reproducible on the master branch, but it certainly blocks development of the canonical synchronization feature (bug 433206) on the committers/cdamus/433206-diagram-sync feature branch.

The CSS engine triggers diagram refresh when something changes, either in application
of style classes to views, stylesheets to diagrams, or the CSS definitions within a stylesheet.  This refresh makes some problems for CanonicalEditPolicy:

* all open diagrams are refreshed, not just the affected diagram.  This is a general
  problem not specific to CanonicalEditPolicy.  Some changes obviously can only
  affect the presentation of a single diagram, so refresh should be scoped
  accordingly

* refresh is invoked asynchronously, for perfectly valid performance reasons.
  The consequence is that CanonicalEditPolicy is invoked in a read-only context
  after the completion of whatever command made changes to the CSS styling of the
  diagram.  That, in turn, means that undoing the CSS change does not undo what
  a CanonicalEditPolicy may have done in creation of views from already existing
  semantic elements.  Undoing further to undo creation of those elements, then,
  breaks the diagram by leaving dangling views

Comment 1 Eclipse Genie

2015-03-06 20:39:12 EST

New Gerrit change created: https://git.eclipse.org/r/43341

Comment 2 Christian Damus

2015-03-10 16:32:58 EDT

I have uploaded a video (referenced in the URL field) demonstrating the consequence of non-transactional CSS-triggered refresh of diagrams when CSS determines the activation of CanonicalEditPolicy:

    http://youtu.be/bv2Gozopha0

Comment 3 Christian Damus

2015-03-11 18:18:53 EDT

I've referenced a commit (f584936) on the committers/cdamus/433206-diagram-sync branch that fixes the problem of undo of semantic-model changes breaking diagrams that had been synchronized by canonical edit-policies that subsequently became disabled by non-transactional changes such as CSS refreshes.

This implements a new "semi-active" state in the PapyrusCanonicalEditPolicy, in which it is not active as far as GMF is concerned, in particularly not creating and deleting child views as necessary to keep in synch with the semantic model.  However, in this semi-active state, it will delete any views that it had previously created by canonical refresh when the corresponding model elements are deleted/invalidated.  This requires that it track the views that were created by canonical refresh while the policy is active.

As this fix is specific to canonical edit-policy, and probably would only ever be an issue at all in that context, I think it's not appropriate to merge it to master until the rest of the bug 433206 feature is merged.

Comment 4 Camille Letavernier

2015-03-12 05:09:08 EDT

What would happen if the undo operations (As shown in the video) occurred after the diagram has been closed? The EditPolicy would certainly be completely removed, and would be unable to clean anything up? 

Unless it can remain semi-active even after it has been disabled/removed from its edit part, due to the listeners on the Notation Model?

Comment 5 Christian Damus

2015-03-12 08:59:24 EDT

(In reply to Camille Letavernier from comment #4)
> What would happen if the undo operations (As shown in the video) occurred
> after the diagram has been closed? The EditPolicy would certainly be
> completely removed, and would be unable to clean anything up? 

Thanks, you have reminded me why my initial implementation of the canonically-created map was stored statically:  because edit-policies come and go as diagrams are opened and closed, but the identity of notation views persists as long as the Papyrus editor is open.  I broke that when (in the interests of performance) I changed the map to be stored per edit policy instance.

Silly me!

Comment 6 Christian Damus

2015-03-12 10:14:07 EDT

A brief video demonstrating the fix, including closing and re-opening the diagram:

    http://youtu.be/XNyl6hlcs08

Comment 7 Christian Damus

2015-03-25 16:24:32 EDT

Fixed by commit 0766262.