Not using ScaledGraphics class degrades quality at higher zoom

[Phillipus]

In order to fix #621 we removed the use of the org.eclipse.draw2d.ScaledGraphics class. However, looking closer at some zoomed diagrams there are now new issues:

1. The outer rectangle of a box is misaligned on right and bottom sides
2. Fonts are not smooth
3. Other artefacts
4. Fonts loaded via Display#loadFont() are not scaled and rendered correctly (see #628)

[Phillipus]

With ScaledGraphics at 800% zoom:

Without ScaledGraphics at 800% zoom:

[Phillipus]

Here's an example of a font zoomed in at 800%:

With ScaledGraphics at 800% zoom:

Without ScaledGraphics at 800% zoom:

[Phillipus]

I'm beginning to think that Draw2d uses ScaledGraphics for a good reason.

What we could do is copy ScaledGraphics and have our own version that we could hack and use that one (we already do that with the Thumbnail class). Caveat - I don't know what needs changing in ScaledGraphics to fix the original font clipping issue.

[Phillipus]

Another solution is to re-instate ScaledGraphics and use the workaround for TextFlow as mentioned here:

https://www.eclipse.org/forums/index.php?t=msg&th=154575&goto=487928&#msg_487928

fTextFlow = new TextFlow() {
    @Override
    protected void paintFigure(Graphics g) {
        g.setClip(getBounds().getCopy().resize((int)(g.getAbsoluteScale()*3), 0));
        super.paintFigure(g);
    }
};

[jbsarrodie]

Another solution is to re-instate ScaledGraphics and use the workaround for TextFlow

No because it doesn't solves the issue (rendering is also ugly and text is still truncated in several cases)

• The outer rectangle of a box is misaligned on right and bottom sides

That's simply because we don't set a borderwidth, ScaledGraphics was introducing an issue that Archi adressed in drawing figures by substracting one pixel out of dimensions, but in fact the real fix is to shrink dimension by one in every directions and draw a 1px border.

• Fonts are not smooth

Are we sure that the underlying graphics is configured as "advanced" and uses antialias ?

FWIW, If I have to choose, I prefer having a non smooth font at 800% than a smooth but truncated one.

• Other artefacts

For relationships, that's in fact exactly the same in SVG output (at least on Linux), so this makes me think that the real issue lies in the figure drawing code, not the graphics itself (it was just invisible when using ScaledGraphics, but I guess for bad reasons)

• Fonts loaded via Display#loadFont() are not scaled and rendered correctly (see #628)

I have to look at this one in detail, but this might be acceptable in the short/mid terme to explain that sideloading fonts is an experimental figure with known limitations which should be hopefuly solved in future release.

[Phillipus]

That's simply because we don't set a borderwidth, ScaledGraphics was introducing an issue that Archi adressed in drawing figures by substracting one pixel out of dimensions, but in fact the real fix is to shrink dimension by one in every directions and draw a 1px border.

If we edit RectangleFigureDelegate to do that, it still causes misalignment when exporting to SVG.

Are we sure that the underlying graphics is configured as "advanced" and uses antialias ?

Draw2D is doing the drawing on the underlying graphics for figures, not us. But yes they are on. Also, for fonts, I think ScaledGraphics is better at scaling fonts than SWTGraphics.

think that the real issue lies in the figure drawing code

I tried different things for connection Figures, but it still looks the same.

[Phillipus]

I don't know what to do about this any more. I tried to improve the figures by adjusting the bounds of the each figure but can never get a solution.

Here's an Application Component as it is now (no ScaledGraphics class):

And here's the SVG export:

Here it is with some x, y, width, height adjustments made in the figure to compensate:

But here's the SVG export:

[Phillipus]

For comparison, here's using ScaledGraphics:

View:

SVG:

[Phillipus]

It seems that ScaledGraphics has some advantages but is getting something wrong when rendering scaled fonts. I think it is getting the distance between characters too wide so the overall width is too wide and this is then being clipped.

[Phillipus]

Here's an experiment setting the figure's line width to 1:

But here's the SVG export:

[Phillipus]

Here's the same experiment setting the line width to 1 but using ScaledGraphics:

View:

SVG:

[Phillipus]

I wondered whether Modelio uses ScaledGraphics so I looked at their source code. They have a class called ScalableFreeformLayeredPane2 which extends ScalableFreeformLayeredPane and they also have the same workaround as we are now using in our ExtendedScalableFreeformLayeredPane:

    protected void paintClientArea(Graphics graphics) {
        if (getChildren().isEmpty()) {
            return;
        } else if (getScale() == 1.0) {
            super.paintClientArea(graphics);
        } else {
            boolean optimizeClip = getBorder() == null || getBorder().isOpaque();
            if (!optimizeClip) {
                graphics.clipRect(getBounds().getShrinked(getInsets()));
            }

            graphics.scale(getScale());

            graphics.pushState();
            paintChildren(graphics);
            graphics.popState();
            graphics.restoreState();
        }
    }

And they also have a comment in that code:

/**
 * Same as {@link ScalableFreeformLayeredPane} but don't use a ScaledGraphics
 * to paint the area, this class sucks at zooming texts.
 */

See https://github.com/ModelioOpenSource/Modelio/blob/master/modelio/app/app.diagram.elements/src/org/modelio/diagram/elements/core/figures/freeform/ScalableFreeformLayeredPane2.java

[Phillipus]

Modelio have a class ZoomableLineBorder which sort of does what we have tried in our experiments - shrinking the bounds. Also note the use of getWidth() % 2 which reduces width and height only for odd widths.

    public void paint(IFigure figure, Graphics graphics, Insets insets) {
        AbstractBorder.tempRect.setBounds(getPaintRectangle(figure, insets));
        if (getWidth() % 2 != 0) {
            AbstractBorder.tempRect.width--;
            AbstractBorder.tempRect.height--;
        }
        AbstractBorder.tempRect.shrink(getWidth() / 2, getWidth() / 2);

        ZoomDrawer.setLineWidth(graphics, getWidth());
        graphics.setLineStyle(getStyle());

        if (getColor() != null) {
            graphics.setForegroundColor(getColor());
        }
        graphics.drawRectangle(AbstractBorder.tempRect);
    }

[Phillipus]

Aha! I remembered something in ExtendedGraphicsToGraphics2DAdaptor:

/**
 * JB fix - decrease width and height by 1 pixel
 */
@Override
public void fillRectangle(int x, int y, int width, int height) {
    Rectangle2D rect = new Rectangle2D.Float(x + transX, y + transY, width - 1, height - 1);

    checkState();
    getGraphics2D().setPaint(getColor(getSWTGraphics().getBackgroundColor()));
    getGraphics2D().fill(rect);
}

/**
 * JB fix - decrease width and height by 1 pixel
 */
@Override
public void fillRoundRectangle(Rectangle rect, int arcWidth, int arcHeight) {
    RoundRectangle2D roundRect = new RoundRectangle2D.Float(rect.x + transX, rect.y + transY, rect.width - 1, rect.height - 1, arcWidth,
            arcHeight);

    checkState();
    getGraphics2D().setPaint(getColor(getSWTGraphics().getBackgroundColor()));
    getGraphics2D().fill(roundRect);
}

These are not needed now.

[Phillipus]

But there are other issues now we are not using ScaledGraphics such as the icon drawing:

That is like that even at 100%

[Phillipus]

It looks like most of the Figures will have to be re-done.

[Phillipus]

I've created a branch figure-fixes with a few fixes for the Figures. There's more to be done.

[Phillipus]

Here's an example of a font zoomed in at 800%:

With ScaledGraphics at 800% zoom:

Without ScaledGraphics at 800% zoom:

I just checked the quality of zoomed fonts in Modelio. Same thing, as they also don't use ScaledGraphics

[Phillipus]

I've done some more fixes for some Figures in the figure-fixes branch.

For simple shapes like rectangles and rounded rectangles this was simply a case of shortening the width and height of a rectangle by 1 pixel both for the fill and for the outline (when we used ScaledGraphics we only did that for the outline).

However, I can't fix where we've used a PointList to draw polygons.

For example, AbstractMotivationFigure uses the same PointList for the fill and for the outline, and yet they are drawn differently:

The ScaledGraphics class has been tuned to draw graphics differently depending on the current scale factor and seems to do a better job. SWTGraphics doesn't do this.

[Phillipus]

It seems that a certain other Eclipse-based tool also has some problems when drawing at higher scale factors:

Ouch.

[jbsarrodie]

For example, AbstractMotivationFigure uses the same PointList for the fill and for the outline, and yet they are drawn differently:

In most of your attempts and test, borderwidth is not set, this is mostly why there are issues when zooming. In fact without a bordersidth set, there are several issues because the physical linewith on the screen is always 1 pixel and never zoomed while it should be.

So the only way I see to fix this is:

• set a 1px border on figure
• shrink the "drawing" dimensions by 1 unit (in all directions). This is the trick to fix the clipping which occures on the border itself
• Check figure drawing code to remove any other dimensions adjustments which were created only to accomodate the fact that borderwidth was not set
• When OK, check SVG related code to remove any other compensation factors introduced

And for an easier way to manage code, I would make sure that we have a private method to draw the figure which does is strictly based on a Rectangle (no shrinking), and then have the public draw method creating a Rectangle based on getting the figure dimensions shrinked by 1 and then calling the private method. This should then allow us to easily change the borderwith by using another value instead of 1 for shrinking (this value being related to the wanted borderwidth).

I did some experiments some weeks ago (which led me to the "BTW I find a way to fix the borderwidth issue" comment) and I was able to adjust borderwidth without impact on zoom (but I did test only on Business Actor at that time).

I'm a bit overloaded atm so can't test or contribute unfortunately :-(

[jbsarrodie]

I just checked the quality of zoomed fonts in Modelio. Same thing, as they also don't use ScaledGraphics

IMHO, such anti-aliasing issues are not a big deal. I think it is preferable (and hardly noticeable at zoom factors less than 200%) than having truncated text at most zoom factors.

[Phillipus]

Shrinking the bounds by 1 pixel will fix the clipping but there is a gap around the figure which causes a gap in the connection (I've shown the selection boxes here to show the overall shrinkage)

[jbsarrodie]

I didn't had this when I did test.

Is it visible even at 100% ?

So you experience clipping if you don't shrink ?

[Phillipus]

Is it visible even at 100% ?

Not really, but it is in SVG.

[Phillipus]

I've made progress on the Figures. I want to get us to where we were before removing ScaledGraphics. And once that is done we can think about setting line widths later.

1. Remove the minus 1 pixel width and height adjustments in ExtendedGraphicsToGraphics2DAdaptor
2. Make adjustments for width and height by minus 1 pixel in some figures
3. Use org.eclipse.swt.graphics.Path for drawing fills instead of org.eclipse.draw2d.geometry.PointList.

The key is to use org.eclipse.swt.graphics.Path for drawing where possible.

Here's an example of AbstractMotivationFigure before these changes. This uses Graphics.fillPolygon(points):

And this one uses Graphics.fillPath(path):

I wrote a utility method to convert the co-ordinates of org.eclipse.draw2d.geometry.PointList into a org.eclipse.swt.graphics.Path.

By doing this we can draw with more accuracy.

All of this is in the figure-fixes branch.

I'll release another beta.

[Phillipus]

Damn.

I've just tested on Mac/Linux and there is a difference in how lines are drawn on Mac/Linux now that we are not using ScaledGraphics. On Mac/Linux all lines have a width of 1 and look like this:

When we used ScaledGraphics it looked like this:

This means that Mac/Linux users will see an uneven outline in the figures.

(Edited to include Linux)

[jbsarrodie]

Does this happen only when zooming? Can you do a screenshot of a figure at 100%, 200% and 300%.

When we used ScaledGraphics it looked like this:

This as no borderwidth. Do you means that even with a borderwidth defined, ScaledGraphics was drawing this ?

For me what you get is the "usual" issue: when drawn, the border is centered on the defined coordinate, thus it spans accross clipping area.

Let's say you have a figure from (10,10) to (20,20). When drawing a 3px border, it ends up being drawn (thats an example and not perfectly accurate information) from (9,9) to (21,21) because there are 3px to draw the inner part of the box will be (11,11) to (19,19).

TBH, it seems that draw2d always put the bottom & right borders inside the clipping area while the top & left borders are "on" the limit of the clipping area. So we have to see when (which borders) to remove what (my guess is floor(borderwidth/2)+1) on which OS (in case it is OS dependant).

[Phillipus]

Does this happen only when zooming?

Yes, at all scales.

What I mean is there is now a difference between the non-specified line width on Windows and Mac. If we don't set the line width on Windows we get that thin line which is always the same at all zoom levels, but on Mac the "default" line width is 1 (equivalent to graphics.setLineWidth(1))

[Phillipus]

I have found a method to draw a rectangle with a specified line width that doesn't require any shrinking. Again, it means that we must do all drawing using org.eclipse.swt.graphics.Path

bounds.width--;
bounds.height--;

float lineWidth = 1;
float offSet = lineWidth / 2;
graphics.setLineWidthFloat(lineWidth);

Path path = new Path(null);
path.moveTo(bounds.x + offSet, bounds.y + offSet);
path.lineTo(bounds.x + bounds.width - offSet, bounds.y + offSet);
path.lineTo(bounds.x + bounds.width - offSet, bounds.y + bounds.height - offSet);
path.lineTo(bounds.x + offSet, bounds.y + bounds.height - offSet);
path.lineTo(bounds.x + offSet, bounds.y);
graphics.drawPath(path);
path.dispose();

1. We reduce width and height by 1 pixel
2. We set the line width to 1 (or 2 or 3)
3. We divide the line width by 2 to get a pixel offset
4. We draw a rectangle using the bounds x, y, width and height BUT add and subtract the offSet for x and y

In the case of a line width of 1, the offset is 0.5. This works for all line widths. And we don't need to use bounds.shrink()

[Phillipus]

I've committed the changes to RectangleFigureDelegate in the figure-fixes branch.

Try it for rectangle figures. It works on Windows and Mac.

[Phillipus]

So it seems to me that if we are going to dispense with using ScaledGraphics and Mac is setting the line width to 1 whether we like it or not, we must now find a way to draw all figures with a line width of 1 on all platforms and no longer use the "magic" line width that we have been using.

Drawing operations that use org.eclipse.swt.graphics.Path have a greater accuracy than those that take integers such as graphics.drawRectangle(). Mixing the two drawing methods lead to misalignments,

[Phillipus]

However, I don't think it will be possible to do this for figures other than rectangles. For example, there is no equivalent Path method of graphics.drawRoundRectangle() (this uses integers not float). It does have addArc() methods but these would have to be calculated.

[Phillipus]

Fuck! I may have found the solution.... (look at the SWTGraphics source code...)

[jbsarrodie]

However, I don't think it will be possible to do this for figures other than rectangles. For example, there is no equivalent Path method of graphics.drawRoundRectangle() (this uses integers not float). It does have addArc() methods but these would have to be calculated.

Where there's Math & Geometry there's a way... with me ;-)

look at the SWTGraphics source code...

But where ?????????

[Phillipus]

But where ?????????

public void translate(float dx, float dy)

[jbsarrodie]

public void translate(float dx, float dy)

Seems like you want to translate(offset, offset) or something similar ?

[Phillipus]

Seems like you want to translate(offset, offset) or something similar ?

Yes. The problem is that in most cases we want x and y to be 0.5 pixel indented. With translate we can do this.

Check out rectangle and rounded rectangle figures in the latest commit in figure-fixes branch ;-)

[Phillipus]

So far, this seems to be working. So hopefully we'll be able to achieve our goal of setting line width...

[Phillipus]

Cancel all of those last three messages (which I have deleted) and slap me round the head. I had hard coded a "1" for line width somewhere...

[jbsarrodie]

[Phillipus]

Right then.

I've re-written the code for all of the Figures (yes, it was a pain in the butt). All figures are now drawn with a line width of 1. It also works when setting the line width to 2 or 3 (any more than that looks silly.)

I've tested all of them at different zoom levels and exporting to SVG.

Let's test this and hope there are no more issues...

All in the figure-fixes branch.

[jbsarrodie]

Great. I'll test tonight !

[Phillipus]

Bear in mind a few small caveats. These are things that we are going to have to accept as a small compromise. The thing is, though, as we are no longer using ScaledGraphics then we have to draw all figures with an actual line width because Mac/Linux are not using the "magic" line width (defaults to 1 and looks terrible).

So our choices are:

1. Go back to using ScaledGraphics and try and fix the text clipping issue
2. Don't use ScaledGraphics and keep the "magic" line width but it looks terrible on Mac
3. Don't use ScaledGraphics and set default line width to 1 and use Graphics#translate() to compensate

Caveat:

• Graphics#translate() is called when setting the line width. This will ensure outlines are placed correctly but it means the fill colour is indented (a bit like calling Bounds#shrink). You won't see this as the outline is drawn on top of the fill, but you will see it if you set the outline opacity to zero. Tough luck on that. I tried to draw the fill before calling Graphics#translate() for the outline but it causes over-spill on some figures and when exporting to SVG.

Here's the code that does it:

bounds.width -= lineWidth;
bounds.height -= lineWidth;
graphics.setLineWidthFloat(lineWidth);
graphics.translate(lineWidth / 2, lineWidth / 2);

[jbsarrodie]

Even without having tested this latest version, I choose option 3 as it is for me what should have been the right behavior from the begining if we didn't had to cope with eclipse/ScaledGraphics issues.

[Phillipus]

I agree. I've tested it and it's OK. Given the nature of Draw2d we will always have to compromise somewhere. I think we do a pretty good job at making the best of what we have to deal with.

A small reminder of how it could be done... ;-)

[Phillipus]

as it is for me what should have been the right behavior from the begining

It only took 10 years... ;-)

[Phillipus]

So our choices are:

1. Go back to using ScaledGraphics and try and fix the text clipping issue
2. Don't use ScaledGraphics and keep the "magic" line width but it looks terrible on Mac
3. Don't use ScaledGraphics and set default line width to 1 and use Graphics#translate() to compensate

Actually, this is a 4th choice:

4. Go back to using ScaledGraphics and fix the text clipping issue and set the default line width to 1 using Graphics#translate()

I only mention it to point out that the line width fix using Graphics#translate() works both with and without ScaledGraphics.

[jbsarrodie]

Actually, this is a 4th choice:

Yes... but no ;-) because it is (almost) impossible to fix the font clipping issue.

I only mention it to point out that the line width fix using Graphics#translate() works both with and without ScaledGraphics.

That's good new though.

[Phillipus]

(I'm not advocating for some of these choices, I just want to record them here so we know why we made these decisions)

BTW - you can actually set line widths to floats like 1.5 and 2.5 and it works.

Suggest this:

1. You test this
2. We issue a new beta
3. If it is all OK we could add a line width section to the Properties view (would need to decide what are reasonable limits, a line width greater than 3 or 4 is not a good look)