Skip to content

Fix #5895: Properly clip MOVETO commands #5911

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 7 commits into from
Feb 22, 2016

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Jan 25, 2016

I haven't run the tests on this yet... we'll see what Travis CI thinks.

@tacaswell tacaswell modified the milestone: Critical bug fix release (1.5.2) Jan 25, 2016
@WeatherGod
Copy link
Member

Travis failed with lots of small differences.

@mdboom
Copy link
Member Author

mdboom commented Jan 27, 2016

This is passing Travis now and ready for final review.

}

while ((code = m_source->vertex(x, y)) != agg::path_cmd_stop) {
if (code == (agg::path_cmd_end_poly | agg::path_flags_close)) {
switch (code) {
case (agg::path_cmd_end_poly | agg::path_flags_close):
Copy link
Member

Choose a reason for hiding this comment

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

what does this line do? My attempts to understand this by writing code have not gone well

// switch_statement1.cpp
#include <iostream>

enum test_e
  {
    A, B, C
  };



void helper(test_e inp)
{
  switch (inp)
  {
  case (A | B):
    std::cout<< "One"<< std::endl;
    break;
  case C:
    std::cout<< "Three"<< std::endl;
    break;

  default:
    std::cout<< "NONE?!"<< std::endl;
  }
}

int main() {
  helper(A);
  helper(B);
  helper(C);

}

gives

22:07 $ ./a.out
NONE?!
One
Three

Copy link
Member

Choose a reason for hiding this comment

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

with some digging I now see that this filtering with two things that just look like they are members of the same enum set.

Copy link
Member Author

Choose a reason for hiding this comment

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

The value agg::path_cmd_end_poly | agg::path_flags_close is how Agg indicates the end of a path that is also closed. Why that's not a single enum value, I don't know, but that's how it's done in Agg so we must follow. If it makes the code clearer I could create a new alias:

end_and_close_poly = agg::path_cmd_end_poly | agg::path_flags_close

Copy link
Member

Choose a reason for hiding this comment

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

Nah, I am just on a wading-into-c-code-I-do-not-understand kick the last few days.

tacaswell added a commit that referenced this pull request Feb 22, 2016
Fix #5895: Properly clip MOVETO commands
@tacaswell tacaswell merged commit 6922209 into matplotlib:master Feb 22, 2016
tacaswell added a commit that referenced this pull request Feb 22, 2016
Fix #5895: Properly clip MOVETO commands
 Conflicts:
	lib/matplotlib/tests/baseline_images/test_axes/log_scales.svg

	kept master version of file
@tacaswell
Copy link
Member

back ported as 512c8d0

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request May 22, 2016
Fix matplotlib#5895: Properly clip MOVETO commands
 Conflicts:
	lib/matplotlib/tests/baseline_images/test_axes/log_scales.svg

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

Successfully merging this pull request may close these issues.

3 participants