Bug 358255

Comment 1 Hernan Gonzalez 2011-09-20 10:38:25 EDT

Created attachment 203685 [details]
implementation of a simple ColorDecorator (border/background)

Comment 2 Hernan Gonzalez 2011-09-20 10:45:22 EDT

Created attachment 203689 [details]
patch to PictogramElementDelegate to support ColorDecorator

Modifications to PictogramElementDelegate  to support ColorDecorator:

1. add the check for null at the end of addDecorators() method. (decorateFigure can return null if it does not create a figure).

if (decorateFigure != null)
  decList.add(decorateFigure);

2. add the folowing before returnin at method decorateFigure() :

 // other (non figure) decorators
 if (decorator instanceof ColorDecorator) {
   ColorDecorator hdecorator = ((ColorDecorator) decorator);
  Color color = getConfigurationProvider().getResourceRegistry().getSwtColor(hdecorator.getR(),
	hdecorator.getG(), hdecorator.getB());
  if (hdecorator.isBorder() && hdecorator.getBorderwidth() > 0) {
    // border
    figure.setBorder(new LineBorder(color, hdecorator.getBorderwidth()));
  } else { // background: only relevant for shapes, not totally filled
    figure.setBackgroundColor(color);
    if (figure instanceof Shape)
	((Shape) figure).setFill(true);
    }
 }

3. Add to refreshFigureColors() method:

	figure.setBorder(null);

Comment 3 Matthias Gorning 2011-10-05 05:18:06 EDT

Created attachment 204576 [details]
My proposal for the new decorator.

.

Comment 4 Matthias Gorning 2011-10-05 05:18:46 EDT

Before I start with the implementation, we should clarify what kind of information this new decorator should provide.

I've attached a new proposal for this new decorator type.  

What do you think about this proposal?

I've also some questions to the background color/fill color:

- When should we apply this fill color?
- What if the figure has a lot of child figures?
- Apply it only to the main figure? 
- What if the figure has a color gradient?

Perhaps the background color only makes sense if we have simple shapes.

Do you think this background color really makes sense or should we remove it from the decorator?

Please, provide me some feedback.

Comment 5 Hernan Gonzalez 2011-10-05 07:54:28 EDT

(In reply to comment #4)
> Before I start with the implementation, we should clarify what kind of
> information this new decorator should provide.
> 
> I've attached a new proposal for this new decorator type.  
> 
> What do you think about this proposal?
> 

I wonder if adding a LineStyle is a good idea. A decorator have no relation with the pictogram hierarchy and its resources (and this should be also clear to the programmer), it should be pure draw2d/SWT. Wouldn't be more clean to SWT.LINE_XXX constants here?

> I've also some questions to the background color/fill color:
> 
> - When should we apply this fill color?
> - What if the figure has a lot of child figures?
> - Apply it only to the main figure? 
> - What if the figure has a color gradient?
> 
> Perhaps the background color only makes sense if we have simple shapes.
> 
> Do you think this background color really makes sense or should we remove it
> from the decorator?
> 

Well, my own main interest to implement this was the background-color :-) The goal was (and so it's working in my code) to highlight some "node" shapes that are drawn with a rounded rectangle inside a main invisible rectangle, with some internal padding (a typical scenario, I'd say): when the background is painted, the padding is highlighted.
Perhaps the documentation should clarify that the colors (and borders!) of this decorator will only be visible if not drawn by the pictograms shapes themselves. As I comment in my snippet, the background is "only relevant for shapes not totally filled".

Comment 6 Michael Wenz 2012-02-23 07:41:51 EST

Comment on attachment 203685 [details]
implementation of a simple ColorDecorator (border/background)

Partly taken over into the Graphiti framework

Comment 7 Michael Wenz 2012-02-23 07:42:15 EST

Comment on attachment 203689 [details]
patch to PictogramElementDelegate to support ColorDecorator

Partly taken over into the Graphiti framework

Comment 8 Michael Wenz 2012-02-23 07:45:34 EST

I have taken over parts of Hernan's proposal (thanks for the contribution!) and added 2 new decorators to Graphiti: Border and Color decorators
Additionally there is an example usage from within the Chess example (when creating move connections the allowed squares are highlighted) a reference in the docu and a test scenario in Sketch that is also used from automated tests.
I checked-in the changes to head and pushed them to Eclipse:
commit eae9ca05a7bf14944489fb06b699c6f5a354c22a
Author: mwenz <michael.wenz@sap.com> 2012-02-23 13:21:30
Committer: mwenz <michael.wenz@sap.com> 2012-02-23 13:38:25
Parent: 3a9be986d1526339eb7846552a54dd05182a9e1e (Bug 352874 - Exports of Diagrams > 3000x3000 px * allow PNG as export format * allow export of max 10k x 10k px for this new format * it seems that the underlying SWT export works fine for large diagrams in this format)
Child: 8109103047552329717bb1a939c113256914b5a0 (formatter)
Branches: origin/master, master

Comment 9 Michael Wenz 2012-04-11 10:54:09 EDT

Bookkeeping: Set target release

Comment 10 Michael Wenz 2012-06-29 04:20:28 EDT

Part of Graphiti 0.9.0 (Eclipse Juno)