-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Make hatch linewidth an rcParam #6198
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,8 @@ patch.facecolor : b | |
patch.edgecolor : k | ||
patch.antialiased : True # render patches in antialiased (no jaggies) | ||
|
||
hatch.linewidth : 1.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did we have 3 (!) different default values for this before? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe 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!). There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes -- the C++ side can not access There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.