pytroll / pycoast

Python package for adding coastlines and borders on raster images
http://pycoast.readthedocs.org/en/latest/
GNU General Public License v3.0
40 stars 20 forks source link

Segmentation fault if providing font in grid via aggdraw and add_overlay_from_dict to save.dataset... #43

Open peters77 opened 4 years ago

peters77 commented 4 years ago

Code Sample, a minimal, complete, and verifiable piece of code

font = aggdraw.Font('blue','/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf',
                    opacity=127, size = 40)
coast = {'coast_dir': '/home/test/software/',
         'color': (0, 0, 0), 'width': 1.5, 'resolution': 'i'}
grid  = {'grid': {'major_lonlat': (10,10), 'minor_lonlat': (2, 2),
        'outline': (255, 255, 0) , 'outline_opacity': 175,
        'width': 1.0, 'minor_width': 0.5, 'minor_is_tick': 1,
        'write_text': True, 'lat_placement': 'lr', 'lon_placement': 'b',
        'font': font}}
deco = [{'text': {'txt': 'MSG4', 
         'font': '/usr/share/fonts/truetype/dejavu/DejaVuSerif.ttf',
         'font_size': 20, 'height': 10, 'bg': 'white', 'bg_opacity': 127, 'line': 'black'}}]

new_scene.save_dataset(composite, imgdir + 'MSG-4.jpg', 
                       overlay = {**coast, **grid},
                       decorate = {'decorate': deco}, fill_value=0)

Problem description

Providing a font with color, type, opacity, and fontsize via aggdraw for grid used in save.dataset via add_overlay_from_dict throws a segmantation fault.

[INFO: 2020-05-25 17:32:26 : satpy.writers] Add coastlines and political borders to image.
Segmentation fault 

This works,

coast = {'coast_dir': '/home/test/software/',
         'color': (0, 0, 0), 'width': 1.5, 'resolution': 'i'}
grid  = {'grid': {'major_lonlat': (10,10), 'minor_lonlat': (2, 2),
        'outline': (255, 255, 0) , 'outline_opacity': 175,
        'width': 1.0, 'minor_width': 0.5, 'minor_is_tick': 1,
        'write_text': True, 'lat_placement': 'lr', 'lon_placement': 'b',                       
        'font': '/usr/share/fonts/truetype/dejavu/DejaVuSerif.ttf', 
        'font_size': 40, 'outline': 'blue'}}
deco = [{'text': {'txt': 'MSG4', 
         'font': '/usr/share/fonts/truetype/dejavu/DejaVuSerif.ttf',
         'font_size': 20, 'height': 10, 'bg': 'white', 'bg_opacity': 127, 'line': 'black'}}]

new_scene.save_dataset(composite, imgdir + 'MSG-4.jpg', 
                       overlay = {**coast, **grid},
                       decorate = {'decorate': deco}, fill_value=0)

...but in https://github.com/pytroll/pycoast/blob/59e981f8cd78e75f543606f9821527a3271fe577/pycoast/cw_base.py#L863-L869 there seems to be a problem setting the opacity (not take into account?) and the font color for the grid labels seems to be the same as for the grid lines (outline)? So I'm not able to provide a different color for the grid lines and the grid lables any more. This was possible before (see font attributes providing via aggdraw) which seems now broken)?

Versions of Python, package at hand and relevant dependencies

aggdraw 1.3.11 pycoast 1.10.3+413.ga5996ec4 satpy 0.10.1.dev2650+gaedf4075 python 3.7 Linux version 4.9.0-9-amd64

djhoese commented 4 years ago

So the main problem I'm noticing with your second case is that you have outline specified twice. Python is probably taking whichever one appears first (or maybe randomly). From what I can tell from the code pycoast has only allowed one color (outline) when the font is provided as a string. I think providing the aggdraw.Font with a different color is the only way this has been possible in the past.

The good news (?) is that I'm able to reproduce the segmentation fault. Not sure what the cause is yet though.

djhoese commented 4 years ago

Ok so I think I figured this out, but the long term fix is not going to be pretty. First, you should no longer provide pycoast options to Satpy the way you are and should try to use the new format that @mraspaud has implemented recently:

coast_dir = '/data/gshhg_shapefiles/'
coast = {'outline': (0, 0, 0), 'width': 1.5, 'resolution': 'i'}
grid  = {'major_lonlat': (10,10), 'minor_lonlat': (2, 2),
         'outline': (255, 255, 0) , 'outline_opacity': 175,
         'width': 1.0, 'minor_width': 0.5, 'minor_is_tick': 1,
         'write_text': True, 'lat_placement': 'lr', 'lon_placement': 'b',
         'font': font, 'font_size': 40}
# this doesn't:
font_file = '/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf'
# font = aggdraw.Font('blue', font_file, size=40)
# grid['font'] = font

# this works (but wrong color):
grid['font'] = font_file
grid['font_size'] = 40

deco = [{'text': {'txt': 'MSG4',
                  'font': '/usr/share/fonts/truetype/dejavu/DejaVuSerif.ttf',
                  'font_size': 20, 'height': 10, 'bg': 'white', 'bg_opacity': 127, 'line': 'black'}}]

scn.save_datasets(filename="C14.jpg", overlay={'coast_dir': coast_dir, 'overlays': {'grid': grid, 'coasts': coast}}, decorate={'decorate': deco}, fill_value=0)

Although kind of ugly (an 'overlays' dict inside an 'overlay' kwarg), it now allows you to pass every option that pycoast supports for each feature being added. Note also how the color for the grid is now outline.

The big reason that this is now seg faulting is that aggdraw Font objects don't seem to be threadsafe. In the last version of Satpy @mraspaud made the overlay functionality "delayed" with dask so it doesn't get computed until you ask it to do it:

    new_image = orig_img.apply_pil(_burn_overlay, res_mode,
                                   None, {'fill_value': fill_value},
                                   (area, cw_, overlays), None)

See here for the rest of the code, but basically apply_pil is running _burn_overlay in a separate thread with dask later on in processing. The aggdraw Font object is contained in overlays and passed to _burn_overlay. It doesn't even get to pycoast before it seg faults. I think I have something that might work as a quick fix in Satpy. I'll make a PR for it, hopefully today.

djhoese commented 4 years ago

So my workaround was going to be to get the properties for the font (size, opacity, file, etc), pass those to the delayed function, then reconstruct the Font object. However, aggdraw's Font objects don't have any properties. I think I'll add the necessary properties to pycoast and hope this works a little better.

Any opinions on font_color versus font_outline?

djhoese commented 4 years ago

@mraspaud It seems there is some difference in defaults and functionality between the from_dict and various add_X methods. It's probably best if all of the keyword arguments were the same. I'm not sure what is best for moving forward on the pycoast side of things. I'd still like to figure out what exactly is causing the segmentation fault with dask.

djhoese commented 4 years ago

More progress: I'm looking at the function here. This is what is actually going to run the pycoast function on the PIL image. I didn't like how the PIL image was created inside the function because pil_image is a Delayed dask object and the function itself is a Delayed function and that seemed risky. Moving that outside of the function didn't help the segmentation fault, but after I did that and I changed pure=True to pure=False it is working. However, I'm not sure why that should effect anything. I am able to do hash(some_font_object) which should be the only real difference between pure=True and pure=False.

Meetings now. Will try to look more later.

djhoese commented 4 years ago

Newest progress that demonstrates why dask hates Font objects:

import aggdraw
font = aggdraw.Font('blue','/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf', opacity=127, size=40)
type(font)

Results in a segmentation fault. I might have a hack to fix it in the trollimage package, but it's not great and is definitely not a fix that should exist in trollimage.

mraspaud commented 4 years ago

I think I was trying to be backwards compatible with the default satpy overlay parameters. But yes, maybe we should bite the bullet and uniformize the defaults to make it pleasing out of the box from both satpy and pycoast

mraspaud commented 4 years ago

So those three lines segfault when run as is or from a delayed function?

djhoese commented 4 years ago

As is. Due to the hacky way aggdraw creates classes/instances a Font "object" isn't really a normal Python object. This is something I want to fix in future versions but kind of need the newest version of the AGG C library to produce the same output we have currently.

The type(font) is reproducing a check that dask is doing to figure out how to serialize an object. It has a multiple dispatch function for tokenizing/normalizing where it says "this object is of type X, let's normalize it as X" for dict, or a sequence, or finally a regular object. Dask can't do this check because type(font_obj) seg faults.

I just got back to working on this, but the possible solution is in trollimage assign a dask_key_name keyword argument when calling the delayed apply function. This is a terrible place to have a fix for this issue since it isn't in aggdraw, satpy, or pycoast, but as far as I can tell is the only place to do it.

djhoese commented 4 years ago

In case people didn't notice, I have a workaround for this at https://github.com/pytroll/trollimage/pull/68

peters77 commented 4 years ago

Dave, I changed the way you provided to add grid via overlays with the new-ish method, but now I get again a problem with grid (with both methods): My code (both ways to provide the font in the code):

font = aggdraw.Font('blue','/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf',
                    size = 40) # Font takes only 4 arguments

grid  = {'grid': {'major_lonlat': (10,10), 'minor_lonlat': (2, 2),
        'outline': (255, 255, 0) , 'outline_opacity': 175,
        'width': 1.0, 'minor_width': 0.5, 'minor_is_tick': 1,
        'write_text': True, 'lat_placement': 'lr', 'lon_placement': 'b',
#Font via aggdraw
#        'font': font}}
#Font inline
        'font': '/usr/share/fonts/truetype/dejavu/DejaVuSerif.ttf', 'font_size': 40, 'outline': 'yellow'}}

coast     = {'outline': (255, 255,   0), 'width': 1.5, 'level': 1, 'resolution': 'i'}
borders   = {'outline': (255,   0,   0), 'width': 1.0, 'level': 1, 'resolution': 'i'}
rivers    = {'outline': (  0,   0, 255), 'width': 1.0, 'level': 3, 'resolution': 'i'}

new_scene.save_dataset(composite, 'MSG-4-euro-raw.jpg',
         overlay = {'coast_dir': '/home/cpeters/software/',
        'overlays': {'grid': grid, 'coasts': coast, 'borders': borders, 'rivers': rivers}}, 
         decorate = {'decorate': deco}, fill_value=0)

I now get:

Traceback (most recent call last):
  File "make_msg4_static_ir_clouds_without_platform.py", line 410, in <module>
    'overlays': {'grid': grid, 'coasts': coast, 'borders': borders, 'rivers': rivers}}, decorate = {'decorate': deco}, fill_value=0)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/satpy/scene.py", line 1306, in save_dataset
    compute=compute, **save_kwargs)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/satpy/writers/__init__.py", line 828, in save_dataset
    decorate=decorate, fill_value=fill_value)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/satpy/writers/__init__.py", line 472, in get_enhanced_image
    img = add_decorate(img, fill_value=fill_value, **decorate)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/satpy/writers/__init__.py", line 387, in add_decorate
    img_orig = orig.pil_image(fill_value=fill_value)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/trollimage/xrimage.py", line 1000, in pil_image
    img = img.compute()
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/dask/base.py", line 166, in compute
    (result,) = compute(self, traverse=False, **kwargs)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/dask/base.py", line 437, in compute
    results = schedule(dsk, keys, **kwargs)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/dask/threaded.py", line 84, in get
    **kwargs
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/dask/local.py", line 486, in get_async
    raise_exception(exc, tb)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/dask/local.py", line 316, in reraise
    raise exc
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/dask/local.py", line 222, in execute_task
    result = _execute_task(task, data)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/dask/core.py", line 121, in _execute_task
    return func(*(_execute_task(a, cache) for a in args))
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/trollimage/xrimage.py", line 644, in _delayed_apply_pil
    new_img = fun(pil_image, image_metadata, *fun_args, **fun_kwargs)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/satpy/writers/__init__.py", line 183, in _burn_overlay
    cw_.add_overlay_from_dict(overlays, area, background=img)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/pycoast/cw_base.py", line 884, in add_overlay_from_dict
    lat_placement=lat_placement)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/pycoast/cw_agg.py", line 364, in add_grid
    lat_placement=lat_placement)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/pycoast/cw_base.py", line 383, in _add_grid
    txt, font, **kwargs)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/pycoast/cw_base.py", line 123, in _draw_grid_labels
    font = self._get_font(kwargs.get('outline', 'black'), font, 12)
  File "/home/cpeters/anaconda2/envs/satpy/lib/python3.7/site-packages/pycoast/cw_agg.py", line 648, in _get_font
    return aggdraw.Font(outline, font_file, size=font_size)
TypeError: Font() argument 2 must be str, not None
djhoese commented 4 years ago

Double check that your final grid dictionary has the font parameter included. The only time I saw this error was when trying to see if adding a grid had a default font (it doesn't) by not providing a font at all. I know the code you pasted looks like it should be there but double check. Something isn't getting all the way through. Maybe a print(grid) to double check.

peters77 commented 4 years ago

Sorry...you are right...Ernst gives me a hint...one gridto much...! :-D This works:


font = aggdraw.Font('blue','/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf',
                    size = 40) # Font takes only 4 arguments

grid  = {{'major_lonlat': (10,10), 'minor_lonlat': (2, 2),
        'outline': (255, 255, 0) , 'outline_opacity': 175,
        'width': 1.0, 'minor_width': 0.5, 'minor_is_tick': 1,
        'write_text': True, 'lat_placement': 'lr', 'lon_placement': 'b',
#Font via aggdraw
#        'font': font}}
#Font inline
        'font': '/usr/share/fonts/truetype/dejavu/DejaVuSerif.ttf', 'font_size': 40, 'outline': 'yellow'}

coast     = {'outline': (255, 255,   0), 'width': 1.5, 'level': 1, 'resolution': 'i'}
borders   = {'outline': (255,   0,   0), 'width': 1.0, 'level': 1, 'resolution': 'i'}
rivers    = {'outline': (  0,   0, 255), 'width': 1.0, 'level': 3, 'resolution': 'i'}

new_scene.save_dataset(composite, 'MSG-4-euro-raw.jpg',
         overlay = {'coast_dir': '/home/cpeters/software/',
        'overlays': {'grid': grid, 'coasts': coast, 'borders': borders, 'rivers': rivers}}, 
         decorate = {'decorate': deco}, fill_value=0)