| Summary: | path operator fixup incorrectly applied to stroked paths in pdf/ps backends | ||
|---|---|---|---|
| Product: | cairo | Reporter: | kellner |
| Component: | pdf backend | Assignee: | Adrian Johnson <ajohnson> |
| Status: | RESOLVED FIXED | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
| Severity: | normal | ||
| Priority: | medium | ||
| Version: | 1.10.0 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | test program | ||
I wrote a test based on your and an hack to fix it here: http://cgit.freedesktop.org/~ranma42/cairo/log/?h=wip/newtests The problem is not _cairo_path_fixed_is_box(), but _cairo_path_fixed_is_rectangle() in cairo-pdf-operators.c The correct fix would probably be to implement a _cairo_path_fixed_iter_is_stroke_box() based on _cairo_path_fixed_iter_is_fill_box() and use them to emit rectangles when targetting ps/pdf, but I'm not sure if it is worth the additional code. I believe we can just emit explicit move_to/line_to commands to describe rectangles in ps/pdf and get rid of _cairo_path_fixed_is_rectangle(). Uh, as usual I was almost forgetting... What is your name? The test has been rewritten (to make it smaller and avoid transparent background), but you found the bug and you should be credited! (In reply to comment #1) > I believe we can just emit explicit move_to/line_to commands to describe > rectangles in ps/pdf and get rid of _cairo_path_fixed_is_rectangle(). It is better to have a few extra lines of code to keep this optimization. It makes a big difference to the PDF file size when there is a very large number of rectangles. eg the test cases in bug 13433. (In reply to comment #3) > (In reply to comment #1) > > I believe we can just emit explicit move_to/line_to commands to describe > > rectangles in ps/pdf and get rid of _cairo_path_fixed_is_rectangle(). > > It is better to have a few extra lines of code to keep this optimization. It > makes a big difference to the PDF file size when there is a very large number > of rectangles. eg the test cases in bug 13433. Currently we only optimize paths composed of a single rectangle. Is this enough or should we also optimize paths composed of multiple rectangles and/or rectangles and other path elements? If we want this optimization in the most generic form (rectangles in a generic ops sequence), we might want to have a fill-path-interpreter and a stroke-path-interpreter which optionally support additional operations, like rectangles. If we just want to optimize multiple-rectangles paths, we probably want _cairo_path_fixed_iter_is_stroke_box() If we just want to optimize paths with a single rectangle, we can fix _cairo_path_fixed_is_rectangle(). (In reply to comment #4) > Currently we only optimize paths composed of a single rectangle. Is this enough > or should we also optimize paths composed of multiple rectangles and/or > rectangles and other path elements? Optimizing single rectangles is probably enough as it is the most common case. The other cases are nice to have but I don't think it is worth spending a lot of time or line of code on them. (In reply to comment #5) > (In reply to comment #4) > > Currently we only optimize paths composed of a single rectangle. Is this enough > > or should we also optimize paths composed of multiple rectangles and/or > > rectangles and other path elements? > > Optimizing single rectangles is probably enough as it is the most common case. > The other cases are nice to have but I don't think it is worth spending a lot > of time or line of code on them. I pushed a commit that fixes the single-rectangle optimization: http://cgit.freedesktop.org/~ranma42/cairo/commit/?h=wip/newtests&id=2e6fe3fc940afe5c8e66741635802df2de7f95a9 The other cases are more complex, so I'll defer optimizing them until they are found to be interesting. (In reply to comment #2) > What is your name? Simon Kellner > The test has been rewritten (to make it smaller and avoid transparent > background), but you found the bug and you should be credited! Ah, I didn't see the test directory when investigating this bug (In reply to comment #7) > (In reply to comment #2) > > What is your name? > Simon Kellner > > > The test has been rewritten (to make it smaller and avoid transparent > > background), but you found the bug and you should be credited! > Ah, I didn't see the test directory when investigating this bug I cleaned up and updated the branch http://cgit.freedesktop.org/~ranma42/cairo/log/?h=wip/newtests The test case now reference you as the author (and is under the MIT licence, which is the default for cairo tests). If this (attribution, licence) is ok for you, I'll push it to master (and probably it is also safe to backport it to 1.10). (In reply to comment #8) > http://cgit.freedesktop.org/~ranma42/cairo/log/?h=wip/newtests > > The test case now reference you as the author (and is under the MIT > licence, which is the default for cairo tests). > > If this (attribution, licence) is ok for you, I'll push it to master (and > probably it is also safe to backport it to 1.10). Both attribution and licence are fine with me. I would have been Ok even with a less prominent attribution. (In reply to comment #9) > (In reply to comment #8) > > http://cgit.freedesktop.org/~ranma42/cairo/log/?h=wip/newtests > > > > The test case now reference you as the author (and is under the MIT > > licence, which is the default for cairo tests). > > > > If this (attribution, licence) is ok for you, I'll push it to master (and > > probably it is also safe to backport it to 1.10). > > Both attribution and licence are fine with me. I would have been Ok > even with a less prominent attribution. Fixed in master by: commit c0fe55651565fa63586b7e4d675149a98c7e549c Author: Andrea Canciani <ranma42@gmail.com> Date: Tue Mar 8 10:26:06 2011 +0100 path: Fix _cairo_path_fixed_is_rectangle() __cairo_path_fixed_is_rectangle() is used by the PS and PDF backends to check if a path is equivalent to a rectangle when stroking. This is different from being a rectangle when filling, because of the implicit close_path appended to every subpath when filling. Fixes stroke-open-box. See https://bugs.freedesktop.org/show_bug.cgi?id=34560 |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Created attachment 43648 [details] test program The function _cairo_path_fixed_is_box() in cairo-path-fixed.c is used in some backends (pdf and ps, maybe more) during a stroke operation. The operator fixup seems to be intended to complete a path prior to filling. When this fixup is also applied to stroke operations, it may cause incorrect output. The test program tries to draw 3 lines. The pdf and ps backends in cairo 1.10.0 (and master branch) draw a rectangle instead, while the svg output is Ok.