MichaelChirico / r-bugs

A ⚠️read-only⚠️mirror of https://bugs.r-project.org/
20 stars 0 forks source link

[BUGZILLA #16299] Line is not cropped correctly in svg() device #5715

Open MichaelChirico opened 4 years ago

MichaelChirico commented 4 years ago

I've noticed that in some cases, a plotted line goes beyond the plot region and the limits of box.

I have identified two examples.

1) When setting xlim to a maximum value lower than the maximum of the data: plot(0:100, xlim=c(1, 10), ylim=c(0, 10), type="l", lwd=15, col="dark grey")

See fig1.svg. Notice how the line goes too far on the right (there's also an artifact above the point where it crosses the box), but not on the left.

2) When setting xaxs="i": plot(0:100, xaxs="i", type="l", lwd=15, col="dark grey")

See fig2.svg. Again, the problem only happens on the right, and not on the left.

3) When calling lines() after plot(), the problem happens on both left and right, and the line is even drawn above the box: plot(0:100, xlim=c(5, 10), ylim=c(0, 10), type="n", lwd=15, col="dark grey") lines(0:100, lwd=15, col="dark grey")

See fig3.svg.

I've seen this with both R 3.1.2 on Windows, and with R 3.1.2 and R-devel on Fedora 21, using either Cairo and Xlib.


METADATA

MichaelChirico commented 4 years ago

Created attachment 1769 [details] fig1.svg


METADATA

INCLUDED PATCH

MichaelChirico commented 4 years ago

Created attachment 1770 [details] fig2.svg


METADATA

INCLUDED PATCH

MichaelChirico commented 4 years ago

Created attachment 1771 [details] fig3.svg


METADATA

INCLUDED PATCH

MichaelChirico commented 4 years ago

I'm no longer able to reproduce the two first issues on Windows with R 3.2.0. But the third one remains, and all three remain on Linux with both R 3.2.0 and R-devel.


METADATA

MichaelChirico commented 4 years ago

I can reproduce this if I use svg(). There's no such phenomenon if I use pdf() and possibly very small / faintly if I use the default interactive device on Linux, x11().

This is about cropping and that is partly device dependent.

I've changed the title of the report (but will not do much more; this is not my field of expertise).


METADATA

MichaelChirico commented 4 years ago

It might be small, but it's still visible with the Xlib/Cairo device. Of course it's more of an issue with SVG since you want high-quality graphics in that case.


METADATA

MichaelChirico commented 4 years ago

Created attachment 1848 [details] non-Cairo version of fig3.svg


METADATA

INCLUDED PATCH

MichaelChirico commented 4 years ago

This may be a Cairo graphics issue (?)

The following code generates a non-Cairo version of fig3 that does NOT exhibit the problem ...

# original code to show problem on Cairo screen device plot(0:100, xlim=c(5, 10), ylim=c(0, 10), type="n", lwd=15, col="dark grey") lines(0:100, lwd=15, col="dark grey")

# generate visually identical grid version that still shows problem # (on Cairo screen device) library(gridGraphics) grid.echo()

# generate non-Cairo SVG version that does NOT show the problem # (file attached) library(gridSVG) grid.export("fig3-export.svg")


METADATA

MichaelChirico commented 4 years ago

I see the same issue in the SVG you attached. The grey line clearly does not stop before the black box.

Also, the bug happens with Xlib, not only Cairo.


METADATA

MichaelChirico commented 4 years ago

... PLUS, according to ...

http://svn.r-project.org/R/trunk/src/library/grDevices/src/cairo/cairoFns.c

... Cairo_Clip() performs a "+ 1" to the clip rect, as per X11_Clip() ...

cairo_new_path(xd->cc);
/* Add 1 per X11_Clip */
cairo_rectangle(xd->cc, x0, y0, x1 - x0 + 1, y1 - y0 + 1);
cairo_clip(xd->cc);

... which is confirmed in ...

http://svn.r-project.org/R/trunk/src/modules/X11/devX11.c

... with this code in X11_Clip() ...

xd->clip.x = (unsigned short) x0 ;
xd->clip.width = (unsigned short) x1 - (unsigned short) x0 + 1;

... BUT Cairo is dealing with doubles, NOT unsigned shorts as in X11. SO Cairo should NOT perform the "+ 1" and should instead act as vector formats like PDF, which ...

http://svn.r-project.org/R/trunk/src/library/grDevices/src/devPS.c

... shows does NOT have the "+ 1" ...

pdfClip(x0, x1, y0, y1, pd);

NOTE that the "+ 1" has some basis for the X11 case because conversion from double to unsigned short drops the fractional part of the double, as stated in ...

https://www.safaribooksonline.com/library/view/c-in-a/0596006977/ch04.html


METADATA

MichaelChirico commented 4 years ago

Created attachment 1851 [details] Screenshot of "non-Cairo version of fig3.svg"

Just to be sure: you don't see the overlapping I'm seeing in the attached screenshot of the SVG you attached?


METADATA

INCLUDED PATCH

MichaelChirico commented 4 years ago

I see exactly that "overlap". The difference is that I think it is correct behaviour. The black line that draws the border on the plot region (and has non-zero width) is centred on the zero-width boundary of the plot region, so that border line extends both inside and outside the plot region (a little bit). The thick grey line is clipped to the plot region, so stops right at the edge of the plot region (half-way through the black line).

The behaviour of the Cairo-device SVG is different. That appears (to me) to extend beyond the plot region (on the right-hand side).


METADATA

MichaelChirico commented 4 years ago

Ah, so now at least we know what we're talking about. :-)

I don't consider this as correct, though. Or maybe that's correct from the backend POV, and I understand how it happens, but it would be nice if the lines never overlapped with the box -- just like what happens when calling plot(.., type="l"), with the box being plotted after the lines.


METADATA

MichaelChirico commented 4 years ago

If you want the box drawn last, you can call box().


METADATA

MichaelChirico commented 4 years ago

Of course I can do that, but I think it should look that way by default...


METADATA