Bug 328801 - filled ellipse renders with small gap
Summary: filled ellipse renders with small gap
Status: NEW
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy Draw2d (show other bugs)
Version: 3.6.1   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: gef-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-27 05:06 EDT by Henrik Rentz-Reichert CLA
Modified: 2015-11-21 04:56 EST (History)
4 users (show)

See Also:


Attachments
image revealing the mentioned gap (2.50 KB, image/gif)
2010-10-27 05:06 EDT, Henrik Rentz-Reichert CLA
no flags Details
some code that may help to reproduce (4.64 KB, text/x-java)
2010-10-27 05:10 EDT, Henrik Rentz-Reichert CLA
no flags Details
pure SWT/draw2d example looks good (1.17 KB, text/plain)
2011-04-04 04:08 EDT, Henrik Rentz-Reichert CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Henrik Rentz-Reichert CLA 2010-10-27 05:06:06 EDT
Created attachment 181809 [details]
image revealing the mentioned gap

This bug was observed in a Graphiti based diagram.

As of the discussion following
http://www.eclipse.org/forums/index.php?t=msg&th=198956&start=0&S=75d2be8d91fb1185adf47d502a7bda94
Tim Kaiser from Graphiti could reproduce it and tracked it back to a problem in the underlying GEF/draw2d layer.

In attached figure the inner GA is a filled ellipse:

Ellipse inset = gaService.createEllipse(invisibleRectangle);
inset.setForeground(manageColor(fill));
inset.setBackground(manageColor(fill));
inset.setLineWidth(LINE_WIDTH);
gaService.setLocationAndSize(inset, 3*size/4, 3*size/4, size/2, size/2);

The rendering leaves a little gap of background color on the upper left
part.

setting the ellipse filled
inset.setFilled(true);
doesn't change the behavior.
Comment 1 Henrik Rentz-Reichert CLA 2010-10-27 05:07:58 EDT
Some additional information:

I'm using Eclipse 3.6, Build id: I20100608-0911
(the 32bit version running on a 64bit Windows 7)

Graphiti 0.7.0
GEF 3.6.1

Debug output:

Pictogram Model Tree()
*PE* ContainerShapeImpl (active) (visible)
  RectangleImpl (360, 0, 80, 80)
    RectangleImpl (20, 20, 40, 40)
    EllipseImpl (30, 30, 20, 20)
  *PE* ShapeImpl (inactive) (visible)
    TextImpl (0, 0, 80, 20)
  *PE* ChopboxAnchorImpl (inactive) (invisible)

Edit Part Tree()
ContainerShapeEditPart (RectangleImpl)

Figure Tree
GFRectangleFigure (Rectangle(400, 40, 80, 80))
  GFRectangleFigure (Rectangle(420, 60, 40, 40))
  GFEllipse (Rectangle(430, 70, 20, 20))
  GFText (text: fcn) (Rectangle(400, 40, 80, 20))
Comment 2 Henrik Rentz-Reichert CLA 2010-10-27 05:10:09 EDT
Created attachment 181810 [details]
some code that may help to reproduce
Comment 3 Alexander Nyßen CLA 2011-03-24 00:04:14 EDT
Henrik, is it possible to come up with a "Draw2d-only" snippet to reproduce the problem? In the thread you are referring to, it is mentioned that Tim had a respective snippet to reproduce the problem.
Comment 4 Henrik Rentz-Reichert CLA 2011-03-28 10:48:10 EDT
(In reply to comment #3)
> Henrik, is it possible to come up with a "Draw2d-only" snippet to reproduce the
> problem? In the thread you are referring to, it is mentioned that Tim had a
> respective snippet to reproduce the problem.

Hi Alexander,

I'm afraid that I can't provide a pure draw2d code.
How can I support you tracking down the problem?
I could offer to make a debug session in my code using some kind of desktop sharing (skype?).

-Henrik
Comment 5 Alexander Nyßen CLA 2011-03-28 11:00:32 EDT
As a starting point, could you provide the complete source code of the class that contains the snippet?
Comment 6 Henrik Rentz-Reichert CLA 2011-04-04 04:08:57 EDT
Created attachment 192438 [details]
pure SWT/draw2d example looks good

Java code trying to resemble the wrong behavior with pure SWT/draw2d code.
But renders as should be.
Where is the difference?
Comment 7 Henrik Rentz-Reichert CLA 2011-04-04 04:17:23 EDT
Tim,

I've brought up this issue in the Graphiti newsgroup.

In
http://www.eclipse.org/forums/index.php?t=tree&goto=634573&S=50b455834c39d76107613a5737e28180#page_top
you mentioned that you could reproduce the issue with plain draw2d, but we couldn't (cf. attachment 3 [details]).

Can you provide this code?
Comment 8 Henrik Rentz-Reichert CLA 2011-04-04 04:20:22 EDT
the 3rd is actually attachment 192438 [details]
Comment 9 Sebastien Delestaing CLA 2015-07-23 04:27:36 EDT
Hi,

this is still a thing. This happens because the outline of the Ellipse is drawn using the scaled rectangle returned by zoomRect while the inner circle is drawn using the rectangle returned by zoomFillrect and those rectangle centers are not aligned.
I believe the bug is in zoomFillRect as it doesn't compensate the (x, y) coordinates.
If I had the proper dev environment I would try to update zoomFillRect from:

private Rectangle zoomFillRect(int x, int y, int w, int h) {
	tempRECT.x = (int) (Math.floor((x * zoom + fractionalX)));
	tempRECT.y = (int) (Math.floor((y * zoom + fractionalY)));
	tempRECT.width = (int) (Math.floor(((x + w - 1) * zoom + fractionalX)))
				- tempRECT.x + 1;
	tempRECT.height = (int) (Math.floor(((y + h - 1) * zoom + fractionalY)))
				- tempRECT.y + 1;
	return tempRECT;
}

to: 

private Rectangle zoomFillRect(int x, int y, int w, int h) {
	tempRECT.x = (int) (Math.floor(((x + 1) * zoom + fractionalX)));
	tempRECT.y = (int) (Math.floor(((y + 1) * zoom + fractionalY)));
	tempRECT.width = (int) (Math.floor(((x + w - 2) * zoom + fractionalX)))
				- tempRECT.x + 1;
	tempRECT.height = (int) (Math.floor(((y + h - 2) * zoom + fractionalY)))
				- tempRECT.y + 1;
	return tempRECT;
}

Sorry, that's how close to a proper patch as I will ever get :)

Regards
Seb
Comment 10 Alexander Nyßen CLA 2015-07-28 14:42:05 EDT
The compensation you refer to within ScaledGraphics is there because of different behavior for draw and fill within SWT (see http://www.eclipse.org/articles/Article-SWT-graphics/SWT_graphics.html). I do not think its causing the issue reported here.
Comment 11 Jules CLERO CLA 2015-10-13 08:40:47 EDT
Fix proposition at: https://github.com/eclipse/gef/pull/2
Comment 12 Alexander Nyßen CLA 2015-11-21 04:56:34 EST
(In reply to Jules CLERO from comment #11)
> Fix proposition at: https://github.com/eclipse/gef/pull/2

I am sorry, but as already indicated, neither this fix nor the one you proposed in comment #9 are correct. The compensation within ScaledGraphics#zoomFillRect() is there because fillRect and drawRect behave differently in SWT (and thus also Draw2d). The problem reported here IMHO is related to scaling effects in combination with integer-based precision (and some inappropriate compensations within Draw2d shapes).

Consider the following snippet that renders the same rectangle with GC (SWT), Graphics (Draw2d), and ScaledGraphics (Draw2d), using the same width and height in all three cases. With the original code base, all three rectangles look the same. Both of your proposed fixes will break the rendering of the last rectangle. 

package swt.bugs;

import org.eclipse.draw2d.ColorConstants;
import org.eclipse.draw2d.Graphics;
import org.eclipse.draw2d.SWTGraphics;
import org.eclipse.draw2d.ScaledGraphics;
import org.eclipse.swt.SWT;
import org.eclipse.swt.events.PaintEvent;
import org.eclipse.swt.events.PaintListener;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Canvas;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;

public class Bug328801 {

	public static void main(String[] args) {
		Display display = new Display();
		Shell shell = new Shell();

		shell.setLayout(new FillLayout());

		Canvas canvas = new Canvas(shell, SWT.NONE);
		canvas.addPaintListener(new PaintListener() {

			public void paintControl(PaintEvent e) {
				GC gc = e.gc;
				gc.setBackground(ColorConstants.red);
				gc.drawRectangle(50, 50, 50, 50);
				gc.fillRectangle(50, 50, 50, 50);

				Graphics graphics = new SWTGraphics(gc);
				graphics.setBackgroundColor(ColorConstants.red);
				graphics.drawRectangle(150, 50, 50, 50);
				graphics.fillRectangle(150, 50, 50, 50);

				ScaledGraphics scaledGraphics = new ScaledGraphics(graphics);
				scaledGraphics.setBackgroundColor(ColorConstants.red);
				scaledGraphics.drawRectangle(250, 50, 50, 50);
				scaledGraphics.fillRectangle(250, 50, 50, 50);
			}
		});

		shell.setSize(400, 200);
		shell.open();

		while (!shell.isDisposed())
			if (!display.readAndDispatch())
				display.sleep();
	}
}