Skip to content
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

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

Open
peters77 opened this issue May 25, 2020 · 13 comments
Labels

Comments

@peters77
Copy link

peters77 commented May 25, 2020

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

pycoast/pycoast/cw_base.py

Lines 863 to 869 in 59e981f

if isinstance(font, str):
if is_agg:
from aggdraw import Font
font = Font(outline, font, size=font_size)
else:
from PIL.ImageFont import truetype
font = truetype(font, font_size)

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

@peters77 peters77 changed the title Segmentation fault if providing font via aggdraw and add_overlay_from_dict to save.dataset... Segmentation fault if providing font in grid via aggdraw and add_overlay_from_dict to save.dataset... May 25, 2020
@djhoese
Copy link
Member

djhoese commented May 26, 2020

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 djhoese added the bug label May 26, 2020
@djhoese
Copy link
Member

djhoese commented May 26, 2020

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
Copy link
Member

djhoese commented May 26, 2020

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
Copy link
Member

djhoese commented May 26, 2020

@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
Copy link
Member

djhoese commented May 26, 2020

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
Copy link
Member

djhoese commented May 26, 2020

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
Copy link
Member

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
Copy link
Member

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

@djhoese
Copy link
Member

djhoese commented May 26, 2020

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
Copy link
Member

djhoese commented May 27, 2020

In case people didn't notice, I have a workaround for this at pytroll/trollimage#68

@peters77
Copy link
Author

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
Copy link
Member

djhoese commented May 27, 2020

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
Copy link
Author

peters77 commented May 27, 2020

Sorry...you are right...Ernst gives me a hint...one grid to 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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants