Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Modeling » Graphiti » Understanding the DiagramEditor and friends internals
Understanding the DiagramEditor and friends internals [message #727732] Wed, 21 September 2011 17:50 Go to next message
Hernan Gonzalez is currently offline Hernan Gonzalez
Messages: 181
Registered: October 2010
Location: Buenos Aires, Argentina
Senior Member
I really don't like that the DiagramEditorInput loads itself the resources (and rather implicitly, at getObject() ). I know that the model is disposed at dispose() , but even then, it sounds wrong and complicates its extensibility (I'm suffering it). I don't think it quite follows the concept of a IEditorInput. But probably I'm lacking perspective.

There seems to be quite a few graphiti objects, for a running editor, tightly coupled and with almost the same lifetime (that of a currently edited diagram).
Lets try some old ASCII art, lets see if I got it right.

                                                              
                       ----->editingDomain <---- DiagramEditorBehavior         -----> (eclipse specific objects)
                      |                 ^         ^                           |
                      |                 |         |                           |
             DiagramEditorInput <---  DiagramEditor <------> configurationProvider  
                      |         (input)        ^                       |
                      |                        |                       |
                      |                 (init) |                       |
                       ---> Diagram <--- diagramTypeProvider <---------
                                             |
                                              ---------> featureProvider
                                             |
                                              ---------> availableToolBehaviorProviders
  
 
                                              
        Graphiti.EmfService.diagToProvider :         Diagram ----->  DiagramTypeProvider
                 (STATIC)   (WEAKHASPMAP)


DiagramEditor - DiagramEditorBehavior (both "internal") have the life of the editor.
Most of the others have a life that coincide with the editing of a particular diagram
(they are instantiated at diagram.setInput)

Now, in theory a single DiagramEditor instance could change its input (calling setInput), so that a single editor could edit (not simultaneously) different diagrams; in practice, I don't see how that can happen; and the full object graph seems, to me, to have essentially the same lifecycle: one instance of each object, while editing a particular diagram (some exceptions, DiagramEditorInput can survive in the Eclipse history); another opened editor has another fully separated object graph.

The diagramTypeProvider is somewhat puzzling, in that the name and the concept suggests only one instance for diagram type; but its init() method (bad name? I don't think
it must be called at object initialiation only) allows it to act as a container of a diagram-editor instance (was this the original idea? sounds a little dirty...).

Questions/Doubts:

- Graphiti.EmfService.diagToProvider looks hackish... can someone explain why is it needed?

- Apart from the simplest lifecycle case: all objects in the above graph are instantiated (one instance of each one) when a diagram is opened in the editor, and die when the editor is closed (expect for the DiagramEditorInput )... are there other possible lifecycles?
Can a DiagramEditor.setInput() be called twice (so that a single DiagramEditor instance is associated with -succesive- distinct Diagram + other objects?)
Can someone explain DiagramEditor.refreshContents() ? I see that it can be called when a Editor regains focus, but, could this trigger a reload of the diagram under any circunstance ?
What is the expected behaviour of the editor if one presses "Refresh" (with and without changes occurred in the file)?


Re: Understanding the DiagramEditor and friends internals [message #728046 is a reply to message #727732] Thu, 22 September 2011 08:55 Go to previous messageGo to next message
Michael Wenz is currently offline Michael Wenz
Messages: 1590
Registered: July 2009
Location: Walldorf, Germany
Senior Member
See my comments below


"Hernan" schrieb im Newsbeitrag news:j5dlhr$r4q$1@news.eclipse.org...

I really don't like that the DiagramEditorInput loads itself the resources
(and rather implicitly, at getObject() ). I know that the model is disposed
at dispose() , but even then, it sounds wrong and complicates its
extensibility (I'm suffering it). I don't think it quite follows the concept
of a IEditorInput. But probably I'm lacking perspective.

Michael: The reason for this was that we wanted to make it more explicit
that the DiagramEditorInput must be fully responsible for the lifetime of
its diagram object and the associated editing domain. That was the original
intention behind, maybe it fainted out over time. Nevertheless, the real
issue behind is making sure that both the diagram object and the editing
domain get disposed when they are no longer needed (the editor is closed)
and making the tool builder aware who is reponsible for the disposing (can
be controlled by the boolean flag in the constructors). If the editing
domain or the diagram are not disposed, each editor input will keep an
instance of the the complete model tree alive causing a huge memory leak
(inputs live rather long for various reasons inside Eclipse). If anyone
comes up with a better approach how to assure that, he's highly welcome.


There seems to be quite a few graphiti objects, for a running editor,
tightly coupled and with almost the same lifetime (that of a currently
edited diagram).
Lets try some old ASCII art, lets see if I got it right.


----->editingDomain <----
ditorBehavior -----> (eclipse specific objects)
| ^ ^
|
| | |
|
DiagramEditorInput <--- DiagramEditor <------>
configurationProvider
| (input) ^ |
| | |
| (init) | |
---> Diagram <--- diagramTypeProvider <---------
|
---------> featureProvider
|
--------->
availableToolBehaviorProviders



Graphiti.EmfService.diagToProvider : Diagram ----->
DiagramTypeProvider
(STATIC) (WEAKHASPMAP)


DiagramEditor - DiagramEditorBehavior (both "internal") have the life of the
editor.
Most of the others have a life that coincide with the editing of a
particular diagram
(they are instantiated at diagram.setInput)

Michael: The original intention behind was to seperate internal behaviour
from behaviour a tool builder should be able to override, but I don't see
that any more. By now I would rather go for having one editor class that
potentially delegates to several "behavior" classes that are exchangeable by
tool builders and care about handling one specific aspect within the editor
like e.g. saving.


Now, in theory a single DiagramEditor instance could change its input
(calling setInput), so that a single editor could edit (not simultaneously)
different diagrams; in practice, I don't see how that can happen; and the
full object graph seems, to me, to have essentially the same lifecycle: one
instance of each object, while editing a particular diagram (some
exceptions, DiagramEditorInput can survive in the Eclipse history); another
opened editor has another fully separated object graph.

Michael: yes, I don't see that in practice either. The reason for having the
setIput method is purely because it is part of the super class interface and
called by a super class during initialization. Replacing the input later
will not work; do you think that shall be prohibited and throw an exception?


The diagramTypeProvider is somewhat puzzling, in that the name and the
concept suggests only one instance for diagram type; but its init() method
(bad name? I don't think
it must be called at object initialiation only) allows it to act as a
container of a diagram-editor instance (was this the original idea? sounds a
little dirty...).

Michael: The original idea was that is should represent one editor instance
(not one editor type) to the tool builder and hide the actual DiagramEditor
class. This turned out to be not possible, since there's too much at the
editor a tool builder might need to change. Today it is an easy way to
influence some aspects of the editor without having to override
DiagramEditor and register a new editor with Eclipse. I think having this
option is good, it might turn out to need some more possibilities.


Questions/Doubts:

- Graphiti.EmfService.diagToProvider looks hackish... can someone explain
why is it needed?

Michael: this is needed to make the connection between a concrete
DiagramEditor and its DiagramTypeProvider known within the core part of the
framework, so that the linking service can access the DiagramtypeProvider
and its ToolBehaviourProvider for resolving linked bussiness objects from
pictogram elements.


- Apart from the simplest lifecycle case: all objects in the above graph are
instantiated (one instance of each one) when a diagram is opened in the
editor, and die when the editor is closed (expect for the
DiagramEditorInput )... are there other possible lifecycles?

Michael: That was the Graphiti idea for the lifecycle, but I'm of course not
sure if all tool builders use it like that.


Can a DiagramEditor.setInput() be called twice (so that a single
DiagramEditor instance is associated with -succesive- distinct Diagram +
other objects?)

Michael: No, see above.


Can someone explain DiagramEditor.refreshContents() ? I see that it can be
called when a Editor regains focus, but, could this trigger a reload of the
diagram under any circunstance ?

Michael: It will refresh the editor either if the loaded diagram object is
still valid but the diagram needs an update to reflect any changed done to
the graphical model, or it will refresh the editor in case the diagram
object has been unloaded (e.g. by a resource changed event cought by the
WorkspaceSynchronizer that is registered by the editor). The idea behing is
that the editor will automatically reload when the resource underneath the
diagram (or a business mode resource) change externally (e.g. triggered by
another editor).


What is the expected behaviour of the editor if one presses "Refresh" (with
and without changes occurred in the file)?

Michael: Is that already answered with the previuos question?
Re: Understanding the DiagramEditor and friends internals [message #728224 is a reply to message #728046] Thu, 22 September 2011 14:38 Go to previous messageGo to next message
Hernan Gonzalez is currently offline Hernan Gonzalez
Messages: 181
Registered: October 2010
Location: Buenos Aires, Argentina
Senior Member
Thank you very much for the detailed answer, I'll be rumiating this...
Re: Understanding the DiagramEditor and friends internals [message #728746 is a reply to message #728046] Fri, 23 September 2011 16:19 Go to previous messageGo to next message
Hernan Gonzalez is currently offline Hernan Gonzalez
Messages: 181
Registered: October 2010
Location: Buenos Aires, Argentina
Senior Member
Michael Wenz wrote on Thu, 22 September 2011 09:55

> I really don't like that the DiagramEditorInput loads itself the resources
(and rather implicitly, at getObject() ). I know that the model is disposed
at dispose() , but even then, it sounds wrong and complicates its
extensibility (I'm suffering it). I don't think it quite follows the concept
of a IEditorInput. But probably I'm lacking perspective.

Michael: The reason for this was that we wanted to make it more explicit
that the DiagramEditorInput must be fully responsible for the lifetime of
its diagram object and the associated editing domain. That was the original
intention behind, maybe it fainted out over time. Nevertheless, the real
issue behind is making sure that both the diagram object and the editing
domain get disposed when they are no longer needed (the editor is closed)
and making the tool builder aware who is reponsible for the disposing (can
be controlled by the boolean flag in the constructors). If the editing
domain or the diagram are not disposed, each editor input will keep an
instance of the the complete model tree alive causing a huge memory leak
(inputs live rather long for various reasons inside Eclipse). If anyone
comes up with a better approach how to assure that, he's highly welcome.


The more I see this, the more I'm convinced this needs rethinking. To prevent those leaks, the more natural way seems to be that the editor cointains the diagram+domain.

That the DiagramEditorInput loads the diagram at construction time seems definetely wrong to me.

Eclipse specifies that DiagramEditorInput should be lightweight, and it relies on this.
I've just experienced an example, reproducible with the Tutorial: create a diagram, and close the eclipse instance (without previously closing the diagram). When you reopen the instance, and the editor reopens itselft with the diagram, eclipse instantiates two DiagramEditorInputs (stacktraces below) which in our case causes the diagram file to be openend and loaded twice. Not a tragedy, but nevertheless, it's ugly.

EditorReference.getRestoredInput() line: 402
EditorReference.getEditorInput() line: 365
EditorReference.createPartHelper() line: 594
EditorReference.createPart() line: 465
EditorReference(WorkbenchPartReference).getPart(boolean) line: 595

NavigationHistoryEditorInfo.restoreEditor() line: 79
NavigationHistory.addEntry(IEditorPart) line: 371
NavigationHistory.access$10(NavigationHistory, IEditorPart) line: 355

I'll probably fill a bug later.
Re: Understanding the DiagramEditor and friends internals [message #731059 is a reply to message #728746] Thu, 29 September 2011 15:39 Go to previous messageGo to next message
Hernan Gonzalez is currently offline Hernan Gonzalez
Messages: 181
Registered: October 2010
Location: Buenos Aires, Argentina
Senior Member
... further, the trick that the EditorDiagramInternal does to switch a (say) FileInputEditor to a DiagramEditorInput is considered bad practice (rightly so, IMHO)

Quote:
While it is tempting to simply replace the editor's input with the wrapped-up input, this should be avoided since it causes trouble e. g. with the problem markers. As a rule of thumb, an editor should always return the same input object that it was initialized with or that was passed to it, no matter what happens internally.


Further, that the very internals of the editor looks sometimes inside the input (eg. inside the DiagramEditorBehavior, to check if "file has been deleted", asking if the "editorInput instanceof DiagramEditorInput" and trying to access the resource) is fatal to extensibility, it makes practically impossible for hte user to implement other storage mechanisms. The editor should be very agnostic here. (The same defect of pretending to know too much about the underlying storage mechanism, and of putting more intelligence in DiagramEditorInput, is the recent addition of a setURI() method for dealing with renaming of files - I don't think that's the right way).


Actually, I'm thinking that the DiagramEditorInput should be ditched. The Diagram input is ... whatever (normally a FileEditorInput) all that the editor needs is someone who provides a Diagram from a given input, and who provides some methods to check for changes, etc. But this provider should be decoupled, and pluggable (could be a factorymethod in the editor, that the user can override with his own provider; or some factory of providers -or providers registry- registered with an extension point).
I was thinking and coding along these lines, until I realized that this idea is already well stablished (IDocumentProvider, in Eclipse API). And I went looking inside GMF code, I see exactly the same idea.

I'm thinking that a port org.eclipse.gmf.runtime.diagram.ui.resources.editor.document package to Graphiti could be highly beneficial to make Graphiti more of a really extensible framework - actually, I'm working on it, at least for my own use.



[Updated on: Thu, 29 September 2011 15:40]

Report message to a moderator

Re: Understanding the DiagramEditor and friends internals [message #735057 is a reply to message #728046] Mon, 10 October 2011 15:48 Go to previous message
Hernan Gonzalez is currently offline Hernan Gonzalez
Messages: 181
Registered: October 2010
Location: Buenos Aires, Argentina
Senior Member
Michael Wenz wrote on Thu, 22 September 2011 09:55

Can someone explain DiagramEditor.refreshContents() ? I see that it can be
called when a Editor regains focus, but, could this trigger a reload of the
diagram under any circunstance ?

Michael: It will refresh the editor either if the loaded diagram object is
still valid but the diagram needs an update to reflect any changed done to
the graphical model, or it will refresh the editor in case the diagram
object has been unloaded (e.g. by a resource changed event cought by the
WorkspaceSynchronizer that is registered by the editor). The idea behing is
that the editor will automatically reload when the resource underneath the
diagram (or a business mode resource) change externally (e.g. triggered by
another editor).



So, we have two editor refresh cases:

In the first one(plain refresh) the editor has the same Diagram object as before (it has not been reloaded - but perhaps it might have been changed in memory). In this scenario (can it be happen with the tutorial? I wonder) this be worked out by GEF with DiagramEditPart and friends.

In the second case (refresh with reload), the editor will have a new Diagram instance, and so it must do much more work (set the model in the graphical viewer, etc), it's almost like a new editor instance (recall that this case can only happen if the editor wasn't dirty, or if the user agreed to loose changes). This can be tested in the tutorial by modifying the current .diagram file with the Eclipse text editor. Some questions about this scenario:

1. Does/should this differ (from the outside; for the graphiti programmer or the user) in anything from openign a new editor instance (in the same window)? Does some editor state or something survives? From what I see, the only relevant object that is reycled is the EditingDomain, though, in the normal workflow this doesn't seem very relevant, it should have its stack flushed.

2. Does this reloading implicitly assumes/requires that the new diagram is "the same"
as the previous one? (Imagine I copied the full xml text from another diagram, perhaps even another diagram type; is the editor expected to break?)

3. (two previous are mere curiosity; this one might be a bug) In the tutorial, open a diagram with a few nodes (EClass) added; with the eclipse text editor modify some attributes of the diagram, the graphics algorithm: for example, change the value that is displayed in the EClass label (don't modify in the business object). When the Graphiti editor refreshes, it correctly reload the diagram but the label is not updated. I'm not sure why. The refresh works ok if, for example, a Eclass is manually added in the editor.


[Updated on: Mon, 10 October 2011 15:50]

Report message to a moderator

Previous Topic:Plans on Graphiti future?
Next Topic:associate keyboard shortcut with custom feature
Goto Forum:
  


Current Time: Mon Sep 01 03:47:25 EDT 2014

Powered by FUDForum. Page generated in 0.02648 seconds