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

Fix some pytest failures on Windows #599

Merged
merged 8 commits into from Apr 30, 2018

Conversation

Projects
None yet
2 participants
@Tontyna
Contributor

Tontyna commented Mar 22, 2018

See #587, UnicodeDecodeError, CairoError

Specify encoding
test_html_parsing cant pass on Windows without specifying utf-8

@Tontyna Tontyna changed the title from Windows default encoding isn't utf-8! to Fix some pytest failures on Windows Mar 22, 2018

@liZe

This comment has been minimized.

Member

liZe commented Mar 22, 2018

Thank you!

It's OK for the UnicodeDecodeError.

For Cairo, I'd really like (if possible) to avoid the crash with font-size: 0 instead of changing tests. Do you get a useful backtrace with these crashes?

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Mar 23, 2018

There are more than 60 snippets using font-size: 0 in the test suite -- and only the one in test_api.test_bookmarks seems to crash Pango. Why? What's so special there? Why doesn't it crash when FreeType rendering is used? I don't get it.

The Pango crash itself is invisible/suppressed in the pytest; test_api.test_bookmark passes, no problem.

Parsing the snippet on its own produces the following bewildering messages:

_cairo_win32_scaled_font_set_metrics:GetTextMetrics: Das System kann die angegebene Datei 
nicht finden.

(python.exe:5168): Pango-WARNING **: failed to create cairo scaled font, expect ugly output. 
the offending font is 'DejaVu Serif Bold 0'

(python.exe:5168): Pango-WARNING **: font_face status is: no error has occurred

 <PageBox PageType(side='right', blank=False, first=True, name=None)> (python.exe:5168): 
Pango-WARNING **: scaled_font status is: error occurred in the Windows Graphics Device Interface
= 643 x 972 @ 0, 0
  _cairo_win32_scaled_font_set_metrics:GetTextMetrics: Der Vorgang wurde erfolgreich beendet.

What? What is Windows trying to tell?
In the first line it complains: "Das System kann die angegebene Datei nicht finden" -- meaning: Cannot find the requested file.
In the last line it has changed its mind: "Der Vorgang wurde erfolgreich beendet" -- Operation successfully completed, thank you very much.
Inbetween Pango, unable to decide whether an error happended or not.

BTW: 'DejaVu Serif Bold 0' can't be ugly because it is invisible.

The induced crashing cairocffi.context.Context produces about 1000 to 2000 log lines per failing test. Dont think that the backtraces are very helpful. Nevertheless, if you want me to, I'll try to make an extract of maybe relevant lines.

I suspect it's another missing DLL, will investigate _cairo_win32_scaled_font_set_metrics.

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Mar 23, 2018

Managed to capture useful error log by parsing the following snippet:

<div>
   prepare crash
</div>
<div style="font-size: 0; break-before:always">
  crash with font-size 0 on page 2
</div>
<div style="break-before:always">
   page 3
</div>

The "prepare crash" is important. Without it -- no problems. Page breaks for proof of speculations.

Output with logging.INFO level:

Step 1 - Fetching and parsing HTML - crashtest.html
C:\...\weasyprint\fonts.py:173: UserWarning: @font-face not supported: Cannot load default config file
  warnings.warn(msg)
Step 3 - Applying CSS
Step 4 - Creating formatting structure
Step 5 - Creating layout - Page 1

Until now: everything is ok/as expected.
On Page 1 no offending font in use. Probably a default fallback font with a size other than zero.

On Page 2 the div with font-size 0 hits Cairo/Pango:

Step 5 - Creating layout - Page 2
_cairo_win32_scaled_font_set_metrics:GetTextMetrics: Der Vorgang wurde erfolgreich beendet.
(python.exe:6656): Pango-WARNING **: failed to create cairo scaled font, expectugly output. the offending font is 'DejaVu Serif 0'
(python.exe:6656): Pango-WARNING **: font_face status is: error occurred in the Windows Graphics Device Interface
(python.exe:6656): Pango-WARNING **: scaled_font status is: error occurred in the Windows Graphics Device Interface

The offending code seems to be located in the function _cairo_win32_scaled_font_set_metrics in the file cairo/src/win32/cairo-win32-font.c. Though not being a C programmer the GDI handling therein looks dubious to me. I suspect, when GetTextMetrics fails it leaves the global(!) static HDC in a critical state (the spoilt Zero-Size-Font isn't deselected).
Cf. my previously posted error log. The messages seem to vary. "Der Vorgang wurde erfolgreich beendet" is the Windows System Message for ERROR_SUCCESS. Probably retrieved by GetLastError. Which -- if not called immediately after the error happened -- returns, yes: the updated last error...

In any case: No reason to stop or crash -- not yet.
Carry on with pagination.

Step 5 - Creating layout - Page 3

The Zero-Font-Size crashes Cairo -- not Pango! -- when we actually try to draw on a surface.
Thats the reason why in the test suite the killer test_api.test_bookmarks seems to pass but all the subsequent tests that call write_pdf, produce this vast amout of CairoErrors.

Step 6 - Drawing
Traceback (most recent call last):
  File "C:\...\weasytest.py", line 280, in execute
    document.write_pdf(target=output)
  File "C:\...\weasyprint\document.py", line 494, in write_pdf
    surface.show_page()
  File "c:\...\python36\lib\contextlib.py", line 99, in __exit__
    self.gen.throw(type, value, traceback)
  File "C:\...\weasyprint\draw.py", line 89, in stacked
    context.restore()
  File "c:\...\python36\lib\site-packages\cairocffi\context.py", line 178, in restore
    self._check_status()
  File "c:\...\python36\lib\site-packages\cairocffi\context.py", line 108, in _check_status
    _check_status(cairo.cairo_status(self._pointer))
  File "c:\...\python36\lib\site-packages\cairocffi\__init__.py", line 79, in _check_status
    raise exception(message, status)
cairocffi.CairoError: cairo returned 41: b'error occurred in the Windows Graphics Device Interface'

That's the short version of the backtrace. Unshortened there are lots of "During handling of the above exception, another exception occurred" only repeating that the <cairocffi.context.Context> is scrambled.

Summary:
Wait for (or contribute to) the next Cairo release. Until then avoid font-size: 0 on Windows when your libfontconfig is inoperative.

Will undo the last commit in this PR.

@Tontyna Tontyna force-pushed the Tontyna:test-api branch from b81752d to 6b5ae05 Mar 23, 2018

@liZe

This comment has been minimized.

Member

liZe commented Mar 23, 2018

Thank you really much, that's an impressive report.

I've had bad times reporting Cairo bugs for corner cases, we should try to find a workaround in WeasyPrint in my opinion.

WeasyPrint/weasyprint/draw.py

Lines 1008 to 1011 in 34e2992

def draw_text(context, textbox, enable_hinting):
"""Draw ``textbox`` to a ``cairo.Context`` from ``PangoCairo.Context``."""
# Pango crashes with font-size: 0
assert textbox.style['font_size']

I don't exactly remember what's been done to avoid text with font-size: 0, but I remember that WeasyPrint used to crash. This assertion and its comment seem to imply that 0-sized text is not drawn anymore. As you said, the problem comes for sure when font metrics for a 0-sized font are required and then some normal text is drawn.

I think that the font metrics are requested when layouts are created. Maybe we should try to prevent the creation of these layouts (they're useless by the way). I'll try to find a quick fix if possible.

@liZe

This comment has been minimized.

Member

liZe commented Mar 24, 2018

Here's a patch that may help:

2 files changed, 12 insertions(+)
weasyprint/css/computed_values.py |  2 ++
weasyprint/text.py                | 10 ++++++++++

modified   weasyprint/css/computed_values.py
@@ -304,6 +304,8 @@ def length(computer, name, value, font_size=None, pixels_only=False):
             result = value.value * font_size * ex_ratio(computer.computed)
         elif unit == 'ch':
             # TODO: cache
+            if font_size == 0:
+                return 0
             layout = text.Layout(
                 context=None, font_size=font_size,
                 style=computer.computed)
modified   weasyprint/text.py
@@ -626,6 +626,9 @@ def first_line_metrics(first_line, text, layout, resume_at, space_collapse,
 class Layout(object):
     """Object holding PangoLayout-related cdata pointers."""
     def __init__(self, context, font_size, style):
+        # Cairo crashes with font-size: 0
+        assert font_size
+
         self.context = context
         hinting = context.enable_hinting if context else False
         self.layout = ffi.gc(
@@ -690,6 +693,8 @@ class Layout(object):
 
     def set_tabs(self, style):
         if isinstance(style['tab_size'], int):
+            if style['font_size'] == 0:
+                width = 0
             layout = Layout(
                 context=self.context, font_size=style['font_size'],
                 style=style)
@@ -838,6 +843,11 @@ def create_layout(text, style, context, max_width, justification_spacing):
         or ``None`` for unlimited width.
 
     """
+    if style['font_size'] == 0:
+        # Cairo crashes with font-size: 0
+        # See https://github.com/Kozea/WeasyPrint/pull/599
+        return Layout(context, 1, style)
+
     text_wrap = style['white_space'] in ('normal', 'pre-wrap', 'pre-line')
     if not text_wrap:
         max_width = None
@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Mar 24, 2018

Youre right, the assertion in draw.draw_text comes too late. Respectively it doesn't help. When the given snippet hits draw_text the first time, it's with the "prepare crash" text, having a font-size: 16. And CAIRO CRASHES.
I'll try the patch tomorrow and report what happens.

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Mar 24, 2018

Why assert something when we can prevent it?

# weasyprint/text.py, line 626 ff
class Layout(object):
    """Object holding PangoLayout-related cdata pointers."""
    def __init__(self, context, font_size, style):
        # Cairo crashes with font-size: 0
        # See https://github.com/Kozea/WeasyPrint/pull/599
        if font_size == 0:
            font_size = 1

No need to alter other source files. One location to catch them all.

Works for the above test case (hitting text.create_layout).
Did one test with width: 10ch, hits computed_values.length with unit == 'ch' -- looks ok.
Did one test with tab-set: 20, hits text.set_tabs -- looks ok.

I'm not as familiar with the code base as you, maybe there are reasons not to do it like I propose?

@liZe

This comment has been minimized.

Member

liZe commented Mar 24, 2018

I'm not as familiar with the code base as you, maybe there are reasons not to do it like I propose?

If you set font-size to 1 and don't catch font-size: 0 everywhere else, you'll draw small letters instead of drawing nothing, and you'll break the layout: text doesn't have the same size when drawn with font-size: 0 and font-size: 1px.

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Mar 24, 2018

You're right, was an error in reasoning.

computed_styles.length should indeed return 0 immediately.
But then, your patch for text.create_layout isn't soo much inerter than mine ;)
And text.set_tabs should be patched with an additional else and indent.

No, this doesn't make me happy. Feels like makeshift.

Considering that this error only happens on Windows when Pango/Cairo switch to Win32 API.
Considering that text rendered with Win32 API is anyway buggy/ugly.
Considering that there will be a wonderful documentation on how to install GDK/FontConfig on Windows.

Considering that, I think using a non-crashing 1px font instead of a 0px font is ok. we should ignore the 0px issue.

Anyway, will implement and test the assert font-size stopgap if you vote for it.

@liZe

This comment has been minimized.

Member

liZe commented Mar 25, 2018

And text.set_tabs should be patched with an additional else and indent.

True.

Considering that this error only happens on Windows when Pango/Cairo switch to Win32 API.
Considering that text rendered with Win32 API is anyway buggy/ugly.
Considering that there will be a wonderful documentation on how to install GDT/FontConfig on Windows.
Considering that, I think we should ignore the 0px issue.

You're right, let's forget about this for now.

(I like the third point 😄)

@liZe

This comment has been minimized.

Member

liZe commented Mar 27, 2018

I've used @font-face for tests, removing the need of installing Ahem. I'll also fix the list of fonts used for some tests depending on the OS.

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Mar 27, 2018

Good idea. I'll run the unit tests with your latest commits. Feedback asap.

@liZe liZe added this to the 43 milestone Mar 27, 2018

@liZe liZe added the bug label Mar 28, 2018

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Mar 31, 2018

Couldn't resist 😁

@Tontyna

This comment has been minimized.

Contributor

Tontyna commented Mar 31, 2018

Uninstalled my Ahem font.

pytest with working fontconfig/FreeType -- no errors.
pytest with WinAPI rendering -- 36 tests fail. No CairoError

Didnt yet check how many tests fail because of unsupported @font-face. Those tests could be decorated with @pytest.mark.skipif(fonts.ZERO_FONTSIZE_CRASHES_CAIRO == True)
Ok, the variable could/should be renamed to CAIRO_SWITCHED_TO_TOY_TEXT_API or
FONTCONFIG_OUT_OF_ORDER

@liZe liZe merged commit 58f1f40 into Kozea:master Apr 30, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Nov 14, 2018

py-weasyprint: Update to 43.
Version 43
----------

Released on 2018-11-09.

Bug fixes:

* `#726 <https://github.com/Kozea/WeasyPrint/issues/726>`_:
  Make empty strings clear previous values of named strings
* `#729 <https://github.com/Kozea/WeasyPrint/issues/729>`_:
  Include tools in packaging

This version also includes the changes from unstable rc1 and rc2 versions
listed below.

Version 43rc2
-------------

Released on 2018-11-02.

**This version is experimental, don't use it in production. If you find bugs,
please report them!**

Bug fixes:

* `#706 <https://github.com/Kozea/WeasyPrint/issues/706>`_:
  Fix text-indent at the beginning of a page
* `#687 <https://github.com/Kozea/WeasyPrint/issues/687>`_:
  Allow query strings in file:// URIs
* `#720 <https://github.com/Kozea/WeasyPrint/issues/720>`_:
  Optimize minimum size calculation of long inline elements
* `#717 <https://github.com/Kozea/WeasyPrint/issues/717>`_:
  Display <details> tags as blocks
* `#691 <https://github.com/Kozea/WeasyPrint/issues/691>`_:
  Don't recalculate max content widths when distributing extra space for tables
* `#722 <https://github.com/Kozea/WeasyPrint/issues/722>`_:
  Fix bookmarks and strings set on images
* `#723 <https://github.com/Kozea/WeasyPrint/issues/723>`_:
  Warn users when string() is not used in page margin


Version 43rc1
-------------

Released on 2018-10-15.

**This version is experimental, don't use it in production. If you find bugs,
please report them!**

Dependencies:

* Python 3.4+ is now needed, Python 2.x is not supported anymore
* Cairo 1.15.4+ is now needed, but 1.10+ should work with missing features
  (such as links, outlines and metadata)
* Pdfrw is not needed anymore

New features:

* `Beautiful website <https://weasyprint.org>`_
* `#579 <https://github.com/Kozea/WeasyPrint/issues/579>`_:
  Initial support of flexbox
* `#592 <https://github.com/Kozea/WeasyPrint/pull/592>`_:
  Support @font-face on Windows
* `#306 <https://github.com/Kozea/WeasyPrint/issues/306>`_:
  Add a timeout parameter to the URL fetcher functions
* `#594 <https://github.com/Kozea/WeasyPrint/pull/594>`_:
  Split tests using modern pytest features
* `#599 <https://github.com/Kozea/WeasyPrint/pull/599>`_:
  Make tests pass on Windows
* `#604 <https://github.com/Kozea/WeasyPrint/pull/604>`_:
  Handle target counters and target texts
* `#631 <https://github.com/Kozea/WeasyPrint/pull/631>`_:
  Enable counter-increment and counter-reset in page context
* `#622 <https://github.com/Kozea/WeasyPrint/issues/622>`_:
  Allow pathlib.Path objects for HTML, CSS and Attachment classes
* `#674 <https://github.com/Kozea/WeasyPrint/issues/674>`_:
  Add extensive installation instructions for Windows

Bug fixes:

* `#558 <https://github.com/Kozea/WeasyPrint/issues/558>`_:
  Fix attachments
* `#565 <https://github.com/Kozea/WeasyPrint/issues/565>`_,
  `#596 <https://github.com/Kozea/WeasyPrint/issues/596>`_,
  `#539 <https://github.com/Kozea/WeasyPrint/issues/539>`_:
  Fix many PDF rendering, printing and compatibility problems
* `#614 <https://github.com/Kozea/WeasyPrint/issues/614>`_:
  Avoid crashes and endless loops caused by a Pango bug
* `#662 <https://github.com/Kozea/WeasyPrint/pull/662>`_:
  Fix warnings and errors when generating documentation
* `#666 <https://github.com/Kozea/WeasyPrint/issues/666>`_,
  `#685 <https://github.com/Kozea/WeasyPrint/issues/685>`_:
  Fix many table layout rendering problems
* `#680 <https://github.com/Kozea/WeasyPrint/pull/680>`_:
  Don't crash when there's no font available
* `#662 <https://github.com/Kozea/WeasyPrint/pull/662>`_:
  Fix support of some align values in tables
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment