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

Bug 247788

Summary:	Make ZoomManager accessible outside of the Zest Hierarchy
Product:	[Tools] GEF	Reporter:	Gerald Rosenberg <gerald>
Component:	GEF-Legacy Zest	Assignee:	Ian Bull <irbull>
Status:	RESOLVED FIXED	QA Contact:
Severity:	enhancement
Priority:	P3	CC:	ahunter.eclipse, lding, milesparker, schlamp, zoltan.ujhelyi
Version:	3.4
Target Milestone:	---
Hardware:	PC
OS:	Windows XP
Whiteboard:

Description Gerald Rosenberg

2008-09-18 03:08:37 EDT

Build ID: I20080617-2000

ViewPart implements IZoomableWorkbenchPart, but GraphViewer#getZoomManager() remains inaccesible to users.

Bug 156286 indicates an intent to make ZoomManager accessible to viewers that extend GraphViewer.  ZoomManager was, however, made internal and the Zest jar restrictions preclude access even with @SuppressWarnings("restriction").

Beyond the fact that ZoomManager offers some great functionality to extensions of GraphViewer, keeping it hidden creates a bit of a paradox in using ZoomContributionViewItem.  Specifically, ZoomContributionViewItem does not provide any way to modify the list of scale factors.  Instead, it depends on ZoomManager, which nicely provides several methods to set the available zoom levels.  Consequently, ZoomContributionViewItem is limited to the hard-coded defaults provided by ZoomManager.

At a minimum, can you please relax the restriction on ZoomManager to "discouraged".  Given that Gef makes its ZoomManager full public, "discouraged" access should be sufficient to prevent unnecessary use in Zest.  Besides, the eclipse core and jdt packages list virtually all of their internal classes as "discouraged".  Zest should have the same policy.

Changing access to "discouraged" introduces no backwards-compatibility problems and is sufficient to warn off unnecessary use consistent with other major parts of the Eclipse platform.

================================
(Long term, it would be better to move ZoomManager out of the internal package.  If there is interest, I would be happy to provide a patch making the required changes. My approach would be to move ZoomManager to public and implement an internal proxy ZoomManager class to preserve backwards-compatibility.)

Comment 1 Miles Parker

2008-11-17 03:19:04 EST

OK, I'm a bit lost here. Isn't ZoomManager simply copying org.eclipse.gef.editparts? Is this simply being done to make ZoomManager inaccessible?

Comment 2 Ian Bull

2008-11-17 12:32:27 EST

No, not really.

It was a copy because we don't actually depend on GEF (we just depend on Draw2D).  I will change it to discouraged for now, and we can look at making it public if necessary.

(In reply to comment #1)
> OK, I'm a bit lost here. Isn't ZoomManager simply copying
> org.eclipse.gef.editparts? Is this simply being done to make ZoomManager
> inaccessible?
>

Comment 3 Miles Parker

2008-11-17 13:43:18 EST

Ah. I had forgotten that there was a design requirement to not make Zest GEF dependent. I wonder if that might be something to reconsider. For my own work I had originally intended to go Draw2D only, as I am not really supporting direct editing of model components, but it turned out that there was so much supported through GEF that I would have had to re implement that it just made sense to use GEF. And performance has been really good -- there doesn't seem to be much additional overhead if any.

Comment 4 Ian Bull

2008-11-17 13:49:08 EST

It was more about letting users run Zest in SWT applications, in this case, you don't need the plug-ins GEF uses.

Also, other than the ZoomManager, Zest doesn't use the GEF code, so we would not get any of the performance improvements that GEF offers.  You are right, if you need editing support, then GEF is the best plug-in for you, but in our case, we were focused on just visualizing (non-editable) graphs.  A hybrid GEF / Zest would be an interesting project (giving you basic graph viewing and editing support), but this is probably doable with GMF too.

(In reply to comment #3)
> Ah. I had forgotten that there was a design requirement to not make Zest GEF
> dependent. I wonder if that might be something to reconsider. For my own work I
> had originally intended to go Draw2D only, as I am not really supporting direct
> editing of model components, but it turned out that there was so much supported
> through GEF that I would have had to re implement that it just made sense to
> use GEF. And performance has been really good -- there doesn't seem to be much
> additional overhead if any.
>

Comment 5 Miles Parker

2008-11-24 15:17:53 EST

Yeah, understood re: GEF / SWT, I think that is a very reasonable decision. A lot of confusion for me comes from having Zest as part of GEF project. I think the hybrid idea is best. GMF is just too heavy-weight. I on't have anything against reliance on EMF -- all of my work is based on it -- but for this I think you really want a pure API approach. As I mentioned in gef newsgroup, there are nice components of Zest that it would be quite easy to have a nice integration with if the API weren't quite so locked down. In fact with this I'm not really sure you would need an explicit integration at all. Perhaps the most flexible approach would be to simply support Adapter Factories. For example if IStylingGraphModel could provide an adaptable "IFigureProvider" and "IConnectorProvider" with defaults, you could allow extension while keeping the current API elegant.

Comment 6 Kai Schlamp

2010-05-09 11:06:53 EDT

Any updates here? Im using the current milestones, but the ZoomManager is still not accessible. Any plans to make it public?

Comment 7 Kai Schlamp

2010-05-09 18:13:17 EDT

Hm, I don't see the big problem here. I simply made AbstractZoomableViewer.getZoomManager() public and move ZoomManager and ZoomListener in the org.eclipse.zest.core.viewers package. Where lies the problem to make that officially?

Comment 8 Ian Bull

2010-05-10 18:46:49 EDT

i have marked it x-internal now.  I'm always worried when changing API, so this isn't an API change, but it should allow you to get access to it. Let me know if this work for you.

Comment 9 Kai Schlamp

2010-05-15 09:23:22 EDT

Thanks Ian, I think that would do the trick.

Comment 10 Ian Bull

2010-05-17 01:16:50 EDT

Since I've added the x-internals, ZoomManager should be accessible now.  I'm going to drop the target milestone for this, as the API change for doing this will likely happen if we push towards a Zest 2.0 API.

Comment 11 Zoltan Ujhelyi

2012-11-27 12:56:05 EST

The ZoomManager is already available in the API in the GEF4/Zest code base. It was already done during the fix of https://bugs.eclipse.org/bugs/show_bug.cgi?id=371152.