Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Linux] Issues in cairocontext.cpp #126

Open
ryukau opened this issue Jan 19, 2020 · 7 comments
Open

[Linux] Issues in cairocontext.cpp #126

ryukau opened this issue Jan 19, 2020 · 7 comments
Assignees

Comments

@ryukau
Copy link

ryukau commented Jan 19, 2020

Hi. I'm currently trying to make VSTGUI works on Linux and found several issues in cairocontext.cpp.

Modified cairocontext.cpp that worked in my environment is available on the link below.

1. No degree to radian conversion in drawArc

CDrawContext::drawArc takes angles in degree. However, drawArc in cairocontext.cpp directly passes degrees to cairo_arc without converting them to radian.

cairo_arc (cr, 0, 0, 1, startAngle1, endAngle2); // line 354

Multiplying 2.0 * M_PI / 360.0 fixed this issue.

cairo_arc (cr, 0, 0, 1, startAngle1 * M_PI / 180.0, endAngle2 * M_PI / 180.0);

2. Scaling factors in drawArc and drawEllipse are incorrect

In current code, numerator and denominator of scaling factor are flipped. Code below is from drawEllipse but it's same in drawArc.

CPoint center = rect.getCenter (); // line 364
cairo_translate (cr, center.x, center.y);
cairo_scale (cr, 2.0 / rect.getWidth (), 2.0 / rect.getHeight ());
cairo_arc (cr, 0, 0, 1, 0, 2 * M_PI);
draw (drawStyle);

Only flipping numerator and denominator didn't solve this issue. I also added cairo_save and cairo_restore to make transform to be local. Also this should make line width to be uniform, according to Ellipses With Uniform Stroke Width from cairo documentation.

cairo_save (cr);
CPoint center = rect.getCenter ();
cairo_translate (cr, center.x, center.y);
cairo_scale (cr, rect.getWidth () / 2.0, rect.getHeight () / 2.0);
cairo_arc (cr, 0, 0, 1, 0, 2 * M_PI);
cairo_restore (cr);
draw (drawStyle);

3. drawPolygon closes line

Here, "close" means it connects last point to first point. Unlike D2DDrawContext::drawPolygon, drawPolygon in cairocontext.cpp closes polygon.

auto& last = polygonPointList.back (); // line 318
cairo_move_to (cr, last.x, last.y);
for (auto& it : polygonPointList)
	cairo_line_to (cr, it.x, it.y);

To make drawPolygon in cairocontext.cpp to behave as same as D2DDrawContext::drawPolygon, I changed above code to the following.

auto& first = polygonPointList.front ();
cairo_move_to (cr, first.x, first.y);
for (auto it = polygonPointList.begin () + 1; it != polygonPointList.end (); ++it)
	cairo_line_to (cr, (*it).x, (*it).y);

4. Translation with ± 0.5

There are some + 0.5 and - 0.5 to slightly translate coordinate. They are causing inconsistency between Windows and Linux.

Following methods are affedted.

  • drawLine
  • drawLines
  • drawRect
  • drawPoint

For example in drawLine:

cairo_move_to (cr, start.x + 0.5, start.y + 0.5); // line 266
cairo_line_to (cr, end.x + 0.5, end.y + 0.5);

These + 0.5 make location of paths to be slightly off. When I removed them, cairo put paths to correct location.

cairo_move_to (cr, start.x, start.y);
cairo_line_to (cr, end.x, end.y);
@jpcima
Copy link
Contributor

jpcima commented Sep 1, 2020

@ryukau I think 0.5 offsets are appropriate, given that cairo's top-left pixel will be located at coordinate (0.5,0.5).
I experience a problem of lines which are not displayed, but the above does not solve it.
Instead black lines would appear as half-opacity grey, which suggests lines are drawn exactly on a mid-pixel coordinate, with one of the pixel lines being clipped.

I am lead to believe the problem has to do with the clipper.

Here it's a label frame rendered in Windows, using vanilla vstgui
Capture du 2020-09-01 20-38-31
Same in linux, top and left line not seen
Capture du 2020-09-01 20-39-45

@ryukau
Copy link
Author

ryukau commented Sep 2, 2020

@jpcima Thanks for pointing out. I didn't know the coordinate convension in cairo. For my reference, it's documented in here.

However, ±0.5 is adding a offset relative to other 2D primitives. See the image below.

translate

With ±0.5 translation, the vertical line is slightly on the right relative to circle and arc.

I guess a solution to this is adding ±0.5 to everything.

@jpcima
Copy link
Contributor

jpcima commented Sep 2, 2020

If this might help, I also discovered a bit ago a problem occurring with the flag -ffast-math.
If this compiler flag is set globally in the project, a wrong computation can happen in CRect::makeIntegral which can make a pixel coordinate off by 1.

It's not an overall solution to all coordinate problems, but one worth to check.

Second to this, it's not only cairocontext.cpp but also cairopath.cpp which have ±0.5 issues.
I read that context adds 0.5 to positions, path subtracts 0.5, which I think doesn't makes lots of sense.
I will try to investigate the detail.

@jpcima
Copy link
Contributor

jpcima commented Sep 2, 2020

I have made a first attempt
sfztools@3be004e

This patch gets me identical drawing to D2D, although my program is not very elaborate.

  • pixel alignment using round (like Quartz), based on context's active transform matrix
  • coordinate +0.5 must apply to strokes only, not fills
  • reduce the rectangle size by 1 (like Quartz)

to do: the paths probably.. all that I tried so far is rectangles and rounded rects.
also we don't know if path is going to be a fill or stroke, so can't apply the 0.5 offset on it. (not sure if we need it)

@ryukau
Copy link
Author

ryukau commented Sep 2, 2020

@jpcima The patch breaks the code using CDrawContext::Transform. On my plugins, everything except texts are translated to somewhere invisible.

I tried the change below to pixelAlign on the patch and it made drawings visible. This change aligns 1px strokes, however 2px strokes are blurred.

inline void roundPixel(const ContextHandle& handle, double x, double y)
{
	cairo_user_to_device(handle, &x, &y);
	x = std::round(x);
	y = std::round(y);
	cairo_device_to_user(handle, &x, &y);
}

CPoint pixelAlign (const ContextHandle& handle, const CPoint& point)
{
		double x = point.x;
		double y = point.y;
		roundPixel (handle, x, y);
		return CPoint(x, y);
}

CRect pixelAlign (const ContextHandle& handle, const CRect& rect)
{
		double left = rect.left;
		double top = rect.top;
		double right = rect.right;
		double bottom = rect.bottom;
		roundPixel (handle, left, top);
		roundPixel (handle, right, bottom);
		return CRect(std::round(left), std::round(top), std::round(right), std::round(bottom));
}

@jpcima
Copy link
Contributor

jpcima commented Sep 2, 2020

This code has a problem
inline void roundPixel(const ContextHandle& handle, double x, double y)
change to:
inline void roundPixel(const ContextHandle& handle, double& x, double& y)

You're right, this conversion has a problem. This seems working here at least, I will update the PR with this change. Thanks

@jpcima
Copy link
Contributor

jpcima commented Oct 13, 2020

@scheffle, I have tried your fix at commit b065e1b. The fills are still misaligned.

To explain the problem I faced, here are rectangle shaped drawn in plain cairo, and source that creates it.
cairo-test.cpp.gz
cairo-align

As the picture shows, adding 0.5 makes strokes perfect, but then it makes fills imperfect.
In the previous PR, I would work around this by adding 0.5 or not, depending if the DrawStyle is filled or not.

This is a quote from the cairo FAQ.
(cf. https://www.cairographics.org/FAQ/#sharp_lines)

The reason that cairo does it this way is so that fills align nicely, at the cost that some strokes do not. It is not possible to set up sample locations in a way so that both fills and strokes with integer coordinates work nicely, so one had to be preferred over the other. One argument in favor of preferring fills is that all fills with integer coordinates align nicely this way. The best that can be done with strokes is to make all even-integer-width strokes align nicely (as they do in cairo) or to make odd-integer-width strokes align (which would then break the fill alignment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants