-
Notifications
You must be signed in to change notification settings - Fork 20
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
Comments
So the main problem I'm noticing with your second case is that you have The good news (?) is that I'm able to reproduce the segmentation fault. Not sure what the cause is yet though. |
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:
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 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:
See here for the rest of the code, but basically |
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 |
@mraspaud It seems there is some difference in defaults and functionality between the |
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 Meetings now. Will try to look more later. |
Newest progress that demonstrates why dask hates Font objects:
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. |
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 |
So those three lines segfault when run as is or from a delayed function? |
As is. Due to the hacky way aggdraw creates classes/instances a The I just got back to working on this, but the possible solution is in trollimage assign a |
In case people didn't notice, I have a workaround for this at pytroll/trollimage#68 |
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):
I now get:
|
Double check that your final |
Sorry...you are right...Ernst gives me a hint...one
|
Code Sample, a minimal, complete, and verifiable piece of code
Problem description
Providing a font with color, type, opacity, and fontsize via aggdraw for grid used in
save.dataset
viaadd_overlay_from_dict
throws a segmantation fault.This works,
...but in
pycoast/pycoast/cw_base.py
Lines 863 to 869 in 59e981f
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
The text was updated successfully, but these errors were encountered: