Bugzilla – Full Text Bug Listing
| Summary: | Make UpdateManager.paint(GC gc) protected | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] GEF | Reporter: | Koen van Dijken <koen.van.dijken> | ||||||||
| Component: | GEF-Legacy Draw2d | Assignee: | Alexander Nyßen <nyssen> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | eclipse, heiko.boettger, nyssen | ||||||||
| Version: | 3.2 | ||||||||||
| Target Milestone: | 3.10.0 (Mars) M1 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Koen van Dijken
We do not want to make this API, so you are going to have to make some kind of arguement for what you are trying to do. This is a new method in 3.2. What were you doing prior to 3.2? This package-visible method calls repairDamage(), which is protected so you can override. The basic problem (i think) is that in a subclass of UpdateManager i cannot get hold of a Graphics object to paint my Figures on when a NativeGraphicsSource is used. That is because UpdateManager.paint() calls repairDamage(), through a call to performUpdate(). In UpdateManager.paint()'s call to performUpdate() the Graphics object is lost. In performUpdate() (which I can override indeed) i receive a Rectangle, and in repairDamage() I cannot get hold of a Graphics object to paint on anymore, because NativeGraphicsSource.getGraphics() returns null. Before rev. 1.25 of LightweightSystem all its paint requests were translated in a call to performUpdate() of the UpdateManager, where, through a call to getGraphics() we could get a Graphics object to paint figures on. Before 3.2.0 i used a slightly modified version of DeferredUpdateManager (as it was at that time) with better performance. Also now, with native painting, it still performs better than DeferredUpdateManager, but then i need a Graphics object someway. I attached the source of my updatemanagers, in the state as i have them now, with a protected UpdateManager.paint() method. Also a screenshot of a situation in which my updatemanager outperforms DeferredUpdateManager. The figures are expensive to draw, because of irregular shapes with holes and transparency. Especialy when changes selection from one corner of the screen to the other corner difference is notable. Created attachment 34321 [details]
ClusteringUpdateManager.java
ClusteringUpdateManager.java
Created attachment 34322 [details]
MinimizingUpdateManager.java
MinimizingUpdateManager.java
Created attachment 34323 [details]
screenshot
screenshot
As a temporary workaround, can you try constructing the FigureCanvas without the DOUBLE_BUFFERED style bit set? This should allow your old code to function normally. I tried creating the FigureCanvas without double buffering and then my old code worked indeed without modifications. I must say that i did it (no double buffering) the dirty way, change it in the code of ScrollingGraphicalViewer itself. I did not try to do it the (a) official way yet, this may turn out to be quite difficult. I noticed some thing when experimenting with double buffering or not: - on my machine the effect of double buffering is neglectable (no speed difference with not using double buffering) - without double buffering, but leaving UpdateManager.paint(GC gc) protected and override it in the way DeferredUpdateManager does, gives the user a much sooner feedback. The screen starts to update immediately. Previously, the screen was updated in a buffer, and then displayed at once. With these settings (no double buffering, protected paint) the screen starts to update immediately. When the entire screen has to be updated, and screen updates take about 1-2 seconds then, this is much better for user experience. User sees immediately something is happening, that makes the wait seem less long. In my case, it would be best if I could tell the ScrollingGraphicalViewer wheteher to use double buffering or not, and have a protected paint method in UpdateManager (or something else which gives the same effect). Koen What about the following code:
ScroklingGraphicalViewer viewer = new ScrollingGraphicalViewer();
viewer.setControl(new FigureCanvas(
composite,
SWT.NONE,
viewer.getLightweightSystem()));
I don't see how this can be done. GraphicalViewerImpl.getLightweightSystem is a protected method, so it has to be done from inside a subclass of GraphicalViewerImpl. Doing this in the constructor of a subclass of GraphicalViewerImpl is also impossible because at that time we don't have a parent composite yet. Doing this in a overridden createControl method is impossible because ScrollingGraphicalViewer.createControl() is final ... Doing it like this LightweightSystem lws = new LightweightSystem(); lws.setUpdateManager(new MinimizingUpdateManager()); FigureCanvas fc = new FigureCanvas(parent, SWT.NONE, lws); graphicalViewer.setControl(fc); in the createPartControl of the editor gives me this exception: java.lang.RuntimeException: Can not set control again once it has been set at org.eclipse.draw2d.SWTEventDispatcher.setControl(SWTEventDispatcher.java:399) at org.eclipse.draw2d.LightweightSystem.setControl(LightweightSystem.java:227) at org.eclipse.gef.ui.parts.GraphicalViewerImpl.hookControl(GraphicalViewerImpl.java:256) at org.eclipse.gef.ui.parts.AbstractEditPartViewer.setControl(AbstractEditPartViewer.java:598) .... > GraphicalViewerImpl. Doing this in the constructor of a subclass of
> GraphicalViewerImpl is also impossible because at that time we don't have a
> parent composite yet. Doing this in a overridden createControl method is
> impossible because ScrollingGraphicalViewer.createControl() is final
You should not have to subclass the viewer. What about when viewer.createControl() is normally called (from the editorpart's createControl). Override in your editorpart, and avoid calling createControl() on the viewer.
Well, I was able to switch off the SWT.DOUBLE_BUFFERING in the way you described. This code snippet is from the EditorPart
--------------------------------------------------
@Override
public void createPartControl(final Composite parent) {
class MyViewer extends ScrollingGraphicalViewer {
MyViewer() {
super();
// do NOT use double buffering
FigureCanvas canvas = new FigureCanvas(parent, SWT.NONE, getLightweightSystem());
super.setControl(canvas);
// installRootFigure();
if (getFigureCanvas() == null)
return;
if (getRootFigure() instanceof Viewport)
getFigureCanvas().setViewport((Viewport)getRootFigure());
else
getFigureCanvas().setContents(getRootFigure());
}
}
// create graphical viewer
if (false) {
graphicalViewer = new SpatialViewer(this);
graphicalViewer.createControl(parent);
} else {
// Test for Bugzilla 126797, comment #10
graphicalViewer = new MyViewer();
// Do not call graphicalViewer.createControl, perform the trick in its constructor
// graphicalViewer.createControl(parent);
}
--------------------------------------------------
Basically, what I did was copy the code of ScrollingGraphicalViewer.createControl to MyViewer's constructor, where I do have a parent Composite, and NOT call createControl from the EditorPart's createPartControl(). I had to 'inline' installRootFigure() because it is private, and call a deprecated getRootFigure() because rootFigure is package visibility.
When I managed to switch off the double buffering this way, I was able to inject my own update manager in the LWS fairly easily (not shown in the snippet above, this is what started this all).
Although now I am able to use a different updatemanager by switching off double-buffering without tweeking the GEF sources this is definitely not a solution, it is a temporary workaround as it is very un-intuitive and it depends on a deprecated method.
Take care of misleading indentation. This is what it should look like
--------------------------------------------------
@Override
public void createPartControl(final Composite parent) {
class MyViewer extends ScrollingGraphicalViewer {
MyViewer() {
super();
// do NOT use double buffering
FigureCanvas canvas = new FigureCanvas(parent,
SWT.NONE, getLightweightSystem());
super.setControl(canvas);
// installRootFigure();
if (getFigureCanvas() == null)
return;
if (getRootFigure() instanceof Viewport)
getFigureCanvas().setViewport((Viewport)getRootFigure());
else
getFigureCanvas().setContents(getRootFigure());
}
} // end of class MyViewer
// create graphical viewer
if (false) {
graphicalViewer = new SpatialViewer(this);
graphicalViewer.createControl(parent);
} else {
// Test for Bugzilla 126797, comment #10
graphicalViewer = new MyViewer();
// Do not call graphicalViewer.createControl, perform the trick in its constructor
// graphicalViewer.createControl(parent);
}
--------------------------------------------------
Please make the paint method protected. Reason: It makes a customized / optimized UpdateManager as it is the DeferredUpdateManager impossible. UpdateManager is an abstract base class. The only concrete extending class (DeferredUpdateManager) needs to override the paint mehtod. So it is more than reasonable any other extension of UpdateManager needs to override this method as well. *** Bug 306919 has been marked as a duplicate of this bug. *** Changed visibility of UpdateManager#paint(GC) from package to protected. Changes pushed to origin/master. Resolving as fixed in 3.10.0M1. |