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

Update GraphicsStream.java #7

Merged
merged 1 commit into from
Jun 30, 2020
Merged

Update GraphicsStream.java #7

merged 1 commit into from
Jun 30, 2020

Conversation

mhschmieder
Copy link
Contributor

Fixed the quadratic to cubic curve "degree elevation" algorithm, which made an incorrect assignment of the degree of a quadratic curve when applying the well-known degree elevation formula for converting to the next degree (cubic in this case).

I spent hours on this to make sure the older code really was in error, doing the equation many different ways to make sure I got the same results each time, and comparing against every implementation I've ever seen for EPS and PDF (SVG supports directly-specific quadratic curves). I have a good feeling for how the coding error happened, as the coordinate transforms WOULD be correct if going to a fourth order curve (and then an extra control point would be needed, still using the same formula).

I augmented the comments but retained the original commenting style and words in order to minimize the overall changes. As the file apparently hadn't been updated since 2015, I also bumped the copyright range to 2020 to cover these new edits.

I did not pull in fixes from other forks, but reviewed them all, and am using the ones that are graphics oriented in my local private branch, but none of them touch the code that I edited tonight so there should be no incompatibilities or conflicts if those other forks by other contributors are merged into the main code base later on.

I have another commit to do tomorrow that is unrelated and isn't part of the graphics handling. I thought it best to decouple the changes in two separate commits, as it is still easy to see the combined changes once the second commit is done.

Fixed the quadratic to cubic curve "degree elevation" algorithm, which made an incorrect assignment of the degree of a quadratic curve when applying the well-known degree elevation formula for converting to the next degree (cubic in this case).
@jfree jfree merged commit 8b48d7e into jfree:master Jun 30, 2020
@jfree
Copy link
Owner

jfree commented Jun 30, 2020

Thanks!

@jfree
Copy link
Owner

jfree commented Jun 30, 2020

I'll make the same change on https://github.com/jfree/jfreepdf (which is the version of this library going forward with JDK11+)

@mhschmieder
Copy link
Contributor Author

I have an unrelated change to make tonight or tomorrow. Should that be done on a separate fork then, as it is about the character encoding of the output vs. the Graphics2D function overrides for PDF?

jfree added a commit to jfree/jfreepdf that referenced this pull request Jul 1, 2020
@jfree
Copy link
Owner

jfree commented Jul 1, 2020

It's fine for me if you want to use the fork you have.

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.

2 participants