Bug 77298

Created attachment 97205 [details]
reproducer

Created attachment 97206 [details]
Example output from the reproducer with missing objects.

Created attachment 97207 [details]
Example output from the reproducer when good.

Created attachment 97220 [details] [review]
suggested fix.

Please double check if num_vertices == 1 should never be done (for PDF).

I am not sure why this was introduced at all. With 1.12.2 the lowest num_vertices value was 4, which assured that all objects are rendered to the PDF.

Created attachment 97247 [details] [review]
Suggested fix. Different approach.

Contributed by Patrick Himmelmann. Now we have something to discuss. Maybe we find a third alternative, so that the logic for raster backends continues to discard small objects, but the logic for vector backends includes everything?

Could somebody review the suggested change there?

This is not really a pdf specific problem. We need to always pass strokes through to the vector backends no matter how thin. I don't know the best way to do this as I am not familiar with that code that these patches modify.

(In reply to Sebastien Bacher from comment #6)
> Could somebody review the suggested change there?

Patrick's version is best, please go ahead with this patch!

@Chris Wilson et all:

I'd like to raise priority on this issue.
We know the issue to exist for several years now on Linux systems. Recently we see more and more Windows systems to suffer from the same issue. The attached patch is simple and fixes the issue. 

Is there anything I can do to summon a cairo maintainer here?

is there anyone wanting to review that one?

Created attachment 118380 [details] [review]
Don't cull thin lines on vector backends

The patch didn't work for my test case and broke many other tests.

Attached is a new patch that seems to fix the bug. The patch ensures a minimum line width when calculating stroke extents on vector backends.

Created attachment 118381 [details] [review]
cairo test for thin lines

This patch contains a cairo test for thin lines.

Created attachment 118385 [details]
attachment-30721-0.html

This seems excessively complex, rather than having the later handler (the
one turning a non-zero float into a zero fixed) be patched instead. I
figure it would be correct for all backends to turn very tiny but non-zero
line widths into the smallest ones they can render.


On Mon, Sep 21, 2015 at 5:18 AM, <bugzilla-daemon@freedesktop.org> wrote:

> *Comment # 12 <https://bugs.freedesktop.org/show_bug.cgi?id=77298#c12> on
> bug 77298 <https://bugs.freedesktop.org/show_bug.cgi?id=77298> from Adrian
> Johnson <ajohnson@redneon.com> *
>
> Created attachment 118381 [details] [review] <https://bugs.freedesktop.org/attachment.cgi?id=118381> [details] <https://bugs.freedesktop.org/attachment.cgi?id=118381&action=edit> [review] <https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=77298&attachment=118381>
> cairo test for thin lines
>
> This patch contains a cairo test for thin lines.
>
> ------------------------------
> You are receiving this mail because:
>
>    - You are watching the QA Contact of the bug.
>
>

(In reply to Bill Spitzak from comment #13)
> Created attachment 118385 [details]
> attachment-30721-0.html
> 
> This seems excessively complex, rather than having the later handler (the
> one turning a non-zero float into a zero fixed) be patched instead. I
> figure it would be correct for all backends to turn very tiny but non-zero
> line widths into the smallest ones they can render.

We don't want to alter the line width on the vector backends. If a 0.001mm line width is specified for pdf that is the line width it should use. My patch only alters the line width when the extents are measured for the purpose of culling 0 width/height objects.

I've commit my patch and testcase.

@Adrian - the commit broke the quartz backend (and based on comments on irc the gl backend too):

  CC       cairo-quartz-surface.lo
cairo-quartz-surface.c:2265:12: error: too few arguments to function call, expected 5, have 4
                         content);
                                ^
./cairoint.h:1335:1: note: '_cairo_surface_init' declared here
cairo_private void
^
./cairo-compiler-private.h:133:27: note: expanded from macro 'cairo_private'
#define cairo_private       cairo_private_no_warn cairo_warn
                            ^
./cairo-compiler-private.h:120:31: note: expanded from macro 'cairo_private_no_warn'
#define cairo_private_no_warn   __attribute__((__visibility__("hidden")))
                                ^
1 error generated.
make[3]: *** [cairo-quartz-surface.lo] Error 1

Should be fixed now.

(In reply to Adrian Johnson from comment #17)
> Should be fixed now.

Thx a lot - build fix confirmed for quartz, quartz-image, xml and tee surface backends.

It seems the GL backend still doesn't compile as a couple uses of _cairo_surface_init weren't updated:

cairo-gl-source.c:76:5: error: too few arguments to function ‘_cairo_surface_init’
cairo-gl-source.c:103:5: error: too few arguments to function ‘_cairo_surface_init’

There's also a similar error for _cairo_path_fixed_approximate_stroke_extents():

cairo-gl-msaa-compositor.c:584:6: error: too few arguments to function ‘_cairo_path_fixed_approximate_stroke_extents’

This issue is not fixed yet, see comments downstream:
https://bugs.launchpad.net/inkscape/+bug/1174909

Could someone have another look at this?

Can you test with git master. There was a fix a couple of weeks ago for thin curved lines:

https://cgit.freedesktop.org/cairo/commit/?id=bec8c7508ebc0f69266f9aebe9903539391c519b

Ah, great! Issue is indeed fixed in current master (132794f6832ea83e2f9a72e11b05080d2cdf80f8).