hholzgra / ocitysmap

Fork of ocitysmap repository at savannah.nongnu.org
Other
37 stars 13 forks source link

Index size calculations are not always correct #52

Open hartmut-mariadb opened 2 years ago

hartmut-mariadb commented 2 years ago

In some cases the number of index columns actually needed to fit all category header and index item lines does not match the pre-calculated estimate we get from _compute_column_occupation() and _compute_columns_split() methods in StreetIndexRenderer

This then leads to hitting an assertion error later when actually rendering the index contents:

Traceback (most recent call last):
  File "/home/maposmatic/maposmatic/scripts/render.py", line 516, in run
    output_count = renderer.render(config, self.job.layout,
  File "/home/maposmatic/ocitysmap/ocitysmap/__init__.py", line 586, in render
    self._render_one(config, tmpdir, renderer_cls,
  File "/home/maposmatic/ocitysmap/ocitysmap/__init__.py", line 691, in _render_one
    renderer.render(surface, dpi, osm_date)
  File "/home/maposmatic/ocitysmap/ocitysmap/layoutlib/single_page_renderers.py", line 589, in render
    self._index_renderer.render(ctx, self._index_area, dpi)
  File "/home/maposmatic/ocitysmap/ocitysmap/indexlib/StreetIndex.py", line 886, in render
     assert actual_n_cols <= rendering_area.n_cols
AssertionError
hholzgra commented 2 years ago

See e.g. https://print.get-map.org/maps/217269

hholzgra commented 2 years ago

The actual problem may be that the index calculations are not wrong in general but fail as they presuppose having to deal with Latin text only. See e.g. the slightly higher index category header bar on the right when Chinese characters are used on the right column:

Schermafbeelding 2022-04-14 om 10 33 19

hholzgra commented 2 years ago

Hypothesis from the previous comment does not hold, seeing same assertion on Latin-only indexes, too, e.g.:

https://print.get-map.org/maps/217317

hholzgra commented 2 years ago

It would probably be better to take the pre-calculation as a first estimate only, and to add one more index column and re-render in case we detect an overflow on the first try ...

hholzgra commented 2 years ago

I'm also not really sure what to make out of this comment in single_page_renderers.py

 # NEVER use ctx.scale() here because otherwise pango will
            # choose different font metrics which may be incompatible
            # with what has been computed by __init__(), which may
            # require more columns than expected !  Instead, we have
            # to trick pangocairo into believing it is rendering to a
            # device with the same default resolution, but with a
            # cairo resolution matching the 'dpi' specified
            # resolution. See
            # index::render::StreetIndexRenederer::render() and
            # comments within.

near https://github.com/hholzgra/ocitysmap/blob/master/ocitysmap/layoutlib/single_page_renderers.py#L578

this is code that clearly pre-dates my involvement with this code base quite a bit :(

hholzgra commented 2 years ago

For now I just removed the assertion so rendering does not fail when overflowing the last reserved index column. It "just" produces visually imperfect results instead for now until a real fix exists.

This kind of exception happened rarely enough that this approach should be OK-ish for now ...