Bugzilla – Full Text Bug Listing
| Summary: | add LineAttribute support which assumes advanced graphics | ||
|---|---|---|---|
| Product: | [Tools] GEF | Reporter: | Randy Hudson <hudsonr> |
| Component: | GEF-Legacy Draw2d | Assignee: | Randy Hudson <hudsonr> |
| Status: | RESOLVED WONTFIX | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | ahunter.eclipse, mgobeil |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
|
Description
Randy Hudson
(In reply to comment #0) > This was discussed briefly on the mailing list. [...] Can you provide a link to this thread? We need an example snippets of code that the exhibit this issue. So you are saying that as soon as someone calls LineAttributes support in SWT, advanced graphics is turned on? (In reply to comment #1) > Can you provide a link to this thread? > > We need an example snippets of code that the exhibit this issue. example code in the e-mail: http://dev.eclipse.org/mhonarc/lists/gef-dev/msg01067.html > So you are saying that as soon as someone calls LineAttributes support in SWT, > advanced graphics is turned on? Yes. I implemented it differently from SWT by design so there would be one and only one recommended way to set and store line attributes.
* As far as I know there is no hard requirement for GEF's Graphics API to match SWT's, and SWT's LineAttributes design was more of a result of their api-change constraints than what makes most sense for clients.
* "unpredictable" results are a standard difficulty with stacking up multiple draw methods to share a single gc's state. If you want an anti-aliased rectangle, you've got to call setAntialias(SWT.ON) at the beginning of your method in case someone's turned it off before you. Each figure is fully responsible for ensuring the preconditions for its draw method; having setLineAttributes() automatically switch to advanced graphics would simply be a convenience for those who happen to want advanced graphics set to true.
* Methods should do the one thing their name implies. setLineAttributes() sets the appearance of a line, setAdvanced() selects a graphics engine with a particular set of capabilities. Although the latter happens to influence the fidelity at which the former is rendered to screen, they are orthogonal concerns, and should be kept separate.
I know the following snippet will make perfect sense to SWT users, but it really shouldn't:
gc.setAdvanced(false);
gc.setLineAttributes(attr);
gc.getAdvanced(); // returns true
...and if Draw2D can provide a cleaner API to its users, I think it should.
* Using a dual advanced/non-advanced API using calls that have switch-to-advanced side effects reintroduces all the same problems with cross-platform development SWT tries to suppress in the first place. A client shouldn't have to write code like this:
float width = 5.0;
if(gc.getAdvanced()) {
gc.setLineAttributes(new LineAttributes(width));
} else {
gc.setLineWidth((int)width);
}
...any more than they should have to write this:
float width = 5.0;
if( System.getProperty("os.name").equals("Windows XP") ) {
gdip.setLineWidth(width);
} else {
cairo.setLineWidth(width);
}
* Perhaps the strongest reason against following SWT, is that SWT will throw an exception if there is a call made to setLineAttributes(attr) on a system where an advanced graphics library isn't available (not to be confused with a case where it is available but disabled). The way I've written Draw2D to handle it, a call to setAdvanced(true) will have no effect (since it, unlike its auto-switch-to-advanced friends has a try/catch block to fail gracefully), and the shape will be drawn with gracefully degraded quality. The SWT way, the shape would blow up the app unless it, and every other shape like it, was coded with the same "if(gc.getAdvanced()) {...}" statement in the previous example, which gets worse when you start setting more than just line width.
> * As far as I know there is no hard requirement for GEF's Graphics API to match
> SWT's
Now you know. There is no such thing as a "cleaner API". There is the same, and there is different. What is currently released is different, and needs to be made the same.
> * Perhaps the strongest reason against following SWT, is that SWT will throw an
> exception if there is a call made to setLineAttributes(attr) on a system where
> an advanced graphics library isn't available (not to be confused with a case
> where it is available but disabled). The way I've written Draw2D to handle it,
> a call to setAdvanced(true) will have no effect (since it, unlike its
> auto-switch-to-advanced friends has a try/catch block to fail gracefully), and
> the shape will be drawn with gracefully degraded quality. The SWT way, the
> shape would blow up the app unless it, and every other shape like it, was coded
> with the same "if(gc.getAdvanced()) {...}" statement in the previous example,
> which gets worse when you start setting more than just line width.
I'm not saying we can't have API which behaves differently, but first we should expose all existing SWT API 1-to-1.
Unlike anti-aliasing, the advanced state of the GC is not something you can turn off. If you did, you'd be painting in the wrong place since the clipping and active transform would be lost.
(In reply to comment #5) > Unlike anti-aliasing, the advanced state of the GC is not something you can > turn off. If you did, you'd be painting in the wrong place since the clipping > and active transform would be lost. > SWTGraphics seems to go through a great deal of trouble to emulate the active transform just to support non-advanced graphics (@see translateX/Y). I'd love to get rid of non-advanced graphics support in Draw2D, but the entire library should go one way or the other, and right now it supports non-advanced graphics. > Now you know. There is no such thing as a "cleaner API". There is the same, > and there is different. What is currently released is different, and needs to > be made the same. You've stated they have to be the same, but for what reason? You can accomplish all the same things with this implementation as SWT's, no functionality is lost. As for the view that there is no such thing as cleaner API, only same and different: a) There is definitely such thing as a cleaner API. Having functions without unnecessary side effects, and not having 3 (setLineWidth, setLineWidthFloat, setLineAttributes) different functions to set the same line width property in slightly different ways, which subtly override one another depending on their call order and other functions' hidden side effects, would be a good start. b) It will only lead to following the status quo instead of moving the project forward. We should present an interface that matches GEF's clients' needs, and adapt it as best we can to the implementations available such as SWT or AWT. For example, both are already on the back-end of GEF's Graphics class in GMF to support printing, and E4's SWT API probably won't even look anything like today's either. Marc has provided more than enough justification and I support his API. (In reply to comment #3) > > I know the following snippet will make perfect sense to SWT users, but it > really shouldn't: > > gc.setAdvanced(false); > gc.setLineAttributes(attr); > gc.getAdvanced(); // returns true > > ...and if Draw2D can provide a cleaner API to its users, I think it should. > Agreed. Marc, I suppose you'd argue that this: graphics.setAdvanced(true); graphics.setLineWidth(1.5f); is "cleaner" than this: graphics.setLineWidth(1.5f); (In reply to comment #6) > You've stated they have to be the same, but for what reason? You can > accomplish all the same things with this implementation as SWT's, no > functionality is lost. You've just answered it. They are functionally equivalent. What is lost is people's ability to reuse their current knowledge/expectations. Anyway, I've changed this to an enhancement for new API allowing clients to make a single call instead of 2. re-closing |