XDGF: replace an elliptical arc with a line when a control point is co-linear with begin and end #601
Conversation
…near, replace an arc with a line
| @@ -40,9 +39,9 @@ public void setDebugAcceptor(ShapeVisitorAcceptor acceptor) { | |||
| } | |||
|
|
|||
| @Override | |||
| protected Path2D drawPath(XDGFShape shape) { | |||
There was a problem hiding this comment.
does this compile? Regardless, I don't think we should change protected method signatures in a patch release. Could you consider adding new private methods and leave the protected methods alone?
There was a problem hiding this comment.
Hey @pjfanning! Good catch. I completely forgot that ShapeRenderer class might be used outside of poi codebase itself.
Can you please check out the last revision? I reverted changing the method signature and preserved its behaviour. Now, it draws all the sections but returns only first drawn path.
|
Hi @pjfanning! Can you please take a look one more time? |
| @@ -0,0 +1,76 @@ | |||
| package org.apache.poi.xdgf.usermodel.section.geometry; | |||
There was a problem hiding this comment.
could you add license headers to the new files?
There was a problem hiding this comment.
Just pushed the changes. Headers are added to this file and TestGeometryUtils.java.
|
thanks - merged |
We've faced an issue with elliptical arcs in VSDX files - when a control point of the elliptic arc is co-linear (i.e. lies on the same line) with begin and end points of the arc, it leads to very huge numbers when calculating coordinates of the center of the arc.
It happens because slopes of two tangents (class
EllipticalArcTolines 198 and 201) are equal to each other which leads to division by zero (or by value that is very near to zero).As the solution it's proposed to check co-linearity before calculating an arc. I added this condition check into
EllipticalArcTo#createEllipticalArcmethod to also cover corner-cases withArcTo(which uses the method under the hood) and withRelEllipticalArcTo.Below you can find an example that high-lights the problem.
And after the fix:
The file itself (just change extension to
.vsdxsince GitHub does not allow to upload VSDX files)Single Icon.zip