Skip to content

When drawing markers, don't set the GraphicsContext alpha. #11105

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

Merged
merged 1 commit into from
Apr 30, 2018

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Apr 23, 2018

Setting the alpha value on the GraphicsContext forcefully applies it to
both the edgecolor and the facecolor, except that a facecolor of
None ignores the alpha.

Instead of requiring this behavior (which needlessly complicates the
implementation of new rendering backends), the alpha value (if any) can
directly be preapplied to the edgecolor and facecolor, the edgecolor can
be applied using gc.set_foreground, and the alpha value never set on the
GraphicsContext.

The change to backend_ps is necessary because we no longer artificially set the linewidth to zero for transparent markers to prevent drawing on the ps backend.

Fixes #11104. Milestoned as 2.2.3 as "fixes a regression".

Also (unintentionally) fixes the SVG part of #10035 (not sure for the pdf part, which may involve a ghostscript bug anyways); updated test images accordingly.

Part of the longstanding effort (#7430) to not have two conflicting sources of alpha...

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer added this to the v2.2.3 milestone Apr 23, 2018
@anntzer anntzer force-pushed the markerfacecolor-none-alpha branch 4 times, most recently from 15b8aa1 to 1cd3f75 Compare April 23, 2018 06:38
ln_color_rgba = self._get_rgba_ln_color()
gc.set_foreground(ln_color_rgba, isRGBA=True)
gc.set_alpha(ln_color_rgba[3])
lc_rgba = mcolors.to_rgba(self._color, self._alpha)
Copy link
Member

Choose a reason for hiding this comment

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

OK, so _alpha overrides the alpha in self._color if its not None. I'm confused if thats what happened before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. As a general rule, an explicit alpha will override one that comes "with the color" (we can always discuss whether that's a good strategy (I think it's a mess) but that's what we have for now). The exception is the one noted in the comment re: markeredgecolor.
(Before, that happened via get_rgba_ln_color, which I had forgotten to remove but now did.)

Setting the alpha value on the GraphicsContext forcefully applies it to
both the edgecolor and the facecolor, *except* that a facecolor of
`None` ignores the alpha.

Instead of requiring this behavior (which needlessly complicates the
implementation of new rendering backends), the alpha value (if any) can
directly be preapplied to the edgecolor and facecolor, the edgecolor can
be applied using gc.set_foreground, and the alpha value never set on the
GraphicsContext.
@anntzer anntzer force-pushed the markerfacecolor-none-alpha branch from 1cd3f75 to 7fbd4c2 Compare April 28, 2018 21:07
@@ -536,7 +536,10 @@ def draw_markers(
ps_cmd = ['/o {', 'gsave', 'newpath', 'translate'] # don't want the translate to be global

lw = gc.get_linewidth()
stroke = lw != 0.0
alpha = (gc.get_alpha()
Copy link
Member

Choose a reason for hiding this comment

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

This effectively treats alpha as a bool for ps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do the same for text.

@tacaswell tacaswell merged commit 213f399 into matplotlib:master Apr 30, 2018
@lumberbot-app
Copy link

lumberbot-app bot commented Apr 30, 2018

There seem to be a conflict, please backport manually

@anntzer anntzer deleted the markerfacecolor-none-alpha branch April 30, 2018 00:32
@anntzer anntzer mentioned this pull request Jul 19, 2018
6 tasks
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Aug 4, 2018
…-alpha

FIX: When drawing markers, don't set the GraphicsContext alpha.
Conflicts:
	lib/matplotlib/lines.py
          - keep aliases (which are implemented automatically on
            master) explicitly in code
@tacaswell
Copy link
Member

backported to v2.2.x as 393c862

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

Successfully merging this pull request may close these issues.

pyplot.plot conflict: markerfacecolor ='none' and alpha
3 participants