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

Make hatch linewidth an rcParam #6198

Merged
merged 4 commits into from Apr 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions doc/users/whats_new/style_changes.rst
Expand Up @@ -35,6 +35,18 @@ Plots
the rcParam ``lines.markersize`` so it is consistent with ``plot(X,
Y, 'o')``. The old value was 20, and the new value is 36 (6^2).

Hatching
````````

- The width of the lines in a hatch pattern is now configurable by the
rcParam `hatch.linewidth`, with a default of 1 point. The old
behavior was different depending on backend:

- PDF: 0.1 pt
- SVG: 1.0 pt
- PS: 1 px
- Agg: 1 px

Plot layout
```````````

Expand Down
7 changes: 7 additions & 0 deletions lib/matplotlib/backend_bases.py
Expand Up @@ -792,6 +792,7 @@ def __init__(self):
self._linewidth = 1
self._rgb = (0.0, 0.0, 0.0, 1.0)
self._hatch = None
self._hatch_linewidth = rcParams['hatch.linewidth']
self._url = None
self._gid = None
self._snap = None
Expand Down Expand Up @@ -1111,6 +1112,12 @@ def get_hatch_path(self, density=6.0):
return None
return Path.hatch(self._hatch, density)

def get_hatch_linewidth(self):
"""
Gets the linewidth to use for hatching.
"""
return self._hatch_linewidth

def get_sketch_params(self):
"""
Returns the sketch parameters for the artist.
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/backends/backend_pdf.py
Expand Up @@ -1182,7 +1182,7 @@ def writeHatches(self):
0, 0, sidelen, sidelen, Op.rectangle,
Op.fill)

self.output(0.1, Op.setlinewidth)
self.output(rcParams['hatch.linewidth'], Op.setlinewidth)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How significant of a performance hit is it to be constantly re-querying this dictionary?

Also, is it the right thing to access the rcParams this late in the draw process? We are pretty bad at being consistent, but I don't recall doing this sort of rcParam access this late anywhere else. What if someone uses an rc context to set up the hatches, but the savefig call is outside that context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point on being consistent, but there is currently no other API to change this value. The main reason for getting this PR in for 2.0 is to put as rcparam in so we can change it and provide a way to get back to the old version. Adding a proper API for changing this can be done later as a new feature and at that point it should be set at artist creation time, not draw time.


# TODO: We could make this dpi-dependent, but that would be
# an API change
Expand Down
3 changes: 2 additions & 1 deletion lib/matplotlib/backends/backend_ps.py
Expand Up @@ -303,6 +303,7 @@ def create_hatch(self, hatch):
if hatch in self._hatches:
return self._hatches[hatch]
name = 'H%d' % len(self._hatches)
linewidth = rcParams['hatch.linewidth']
self._pswriter.write("""\
<< /PatternType 1
/PaintType 2
Expand All @@ -313,7 +314,7 @@ def create_hatch(self, hatch):

/PaintProc {
pop
0 setlinewidth
%(linewidth)f setlinewidth
""" % locals())
self._pswriter.write(
self._convert_path(Path.hatch(hatch), Affine2D().scale(72.0),
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/backends/backend_svg.py
Expand Up @@ -393,7 +393,7 @@ def _write_hatches(self):
style=generate_css({
'fill': rgb2hex(stroke),
'stroke': rgb2hex(stroke),
'stroke-width': '1.0',
'stroke-width': six.text_type(rcParams['hatch.linewidth']),
'stroke-linecap': 'butt',
'stroke-linejoin': 'miter'
})
Expand Down
2 changes: 2 additions & 0 deletions lib/matplotlib/mpl-data/stylelib/classic.mplstyle
Expand Up @@ -32,6 +32,8 @@ patch.facecolor : b
patch.edgecolor : k
patch.antialiased : True # render patches in antialiased (no jaggies)

hatch.linewidth : 1.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we have 3 (!) different default values for this before?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually 4, since SVG was "1 point" and Agg was "1 pixel". We could try to emulate that old brokenness if we want (I guess).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I think unify on one is an ok complete break. We should at least document what the old values were. On the off chance we get someone who really cares, they likely only care on one backend (or we would have heard from them sooner!).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I've documented the old values in the style changes what's new.


hist.bins : 10

### FONT
Expand Down
3 changes: 3 additions & 0 deletions lib/matplotlib/rcsetup.py
Expand Up @@ -870,6 +870,9 @@ def validate_animation_writer_path(p):
'patch.facecolor': ['#1f77b4', validate_color], # blue (first color in color cycle)
'patch.antialiased': [True, validate_bool], # antialiased (no jaggies)

## hatch props
'hatch.linewidth': [1.0, validate_float],

## Histogram properties
'hist.bins': [10, validate_hist_bins],

Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified lib/matplotlib/tests/baseline_images/test_colorbar/double_cbar.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions matplotlibrc.template
Expand Up @@ -109,6 +109,9 @@ backend : $TEMPLATE_BACKEND
#patch.edgecolor : black
#patch.antialiased : True # render patches in antialiased (no jaggies)

### HATCHES
#hatch.linewidth : 1.0

### Boxplot
#boxplot.notch : False
#boxplot.vertical : True
Expand Down
2 changes: 1 addition & 1 deletion src/_backend_agg.h
Expand Up @@ -363,7 +363,7 @@ RendererAgg::_draw_path(path_t &path, bool has_clippath, const facepair_t &face,
hatch_path_trans_t hatch_path_trans(hatch_path, hatch_trans);
hatch_path_curve_t hatch_path_curve(hatch_path_trans);
hatch_path_stroke_t hatch_path_stroke(hatch_path_curve);
hatch_path_stroke.width(1.0);
hatch_path_stroke.width(points_to_pixels(gc.hatch_linewidth));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, here in AGG we have the graphics context holding this new parameter, but in the vector backends we don't use the graphics context for that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes -- the C++ side can not access rcParams directly, so we have to stuff it somewhere to pass to _backend_agg.so.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no public API to change the hatch yet and it looks like vector backends do not have the proper hooks set up for the relevant functions to have a reference to the gc.

hatch_path_stroke.line_cap(agg::square_cap);

// Render the path into the hatch buffer
Expand Down
1 change: 1 addition & 0 deletions src/_backend_agg_basic_types.h
Expand Up @@ -108,6 +108,7 @@ class GCAgg
e_snap_mode snap_mode;

py::PathIterator hatchpath;
double hatch_linewidth;

SketchParams sketch;

Expand Down
1 change: 1 addition & 0 deletions src/py_converters.cpp
Expand Up @@ -481,6 +481,7 @@ int convert_gcagg(PyObject *pygc, void *gcp)
convert_from_method(pygc, "get_clip_path", &convert_clippath, &gc->clippath) &&
convert_from_method(pygc, "get_snap", &convert_snap, &gc->snap_mode) &&
convert_from_method(pygc, "get_hatch_path", &convert_path, &gc->hatchpath) &&
convert_from_method(pygc, "get_hatch_linewidth", &convert_double, &gc->hatch_linewidth) &&
convert_from_method(pygc, "get_sketch_params", &convert_sketch_params, &gc->sketch))) {
return 0;
}
Expand Down