Post without Account — your post will be reviewed, and if appropriate, posted under Anonymous.

RT 98541 - bogen() handling edge cases

  • 1 Replies
  • 668 Views
*

Offline Phil

  • Global Moderator
  • Sr. Member
  • *****
  • 356
    • View Profile
RT 98541 - bogen() handling edge cases
« October 20, 2016, 07:54:14 PM »
Subject:    bogen() handling edge cases
Date:    Tue, 02 Sep 2014 11:19:25 -0400
To:    bug-PDF-API2 [...] rt.cpan.org
 
PDF::API2 v2.022   Perl 5.16.3  Windows 7   severity: Wishlist

There is at least one error case for Content.pm's bogen() method. If the radius ($r) is less than half the distance between the two points, this is documented as an error condition, but there appears to be nothing in the code to handle it. Normally, $r should be greater than half the distance between the points, so as to produce four arcs. The limiting case of $r exactly equal to half the distance degenerates to a semicircle between the points. How exactly should bogen() handle $r that is too small? Should it fail in some manner, or treat it as exactly one half the distance? I have updated the POD to clarify the minimum size for $r, and submitted it to the maintainer, but the code should probably be updated to handle this situation.
#
Subject:    [rt.cpan.org #98541]
Date:    Sat, 23 Jan 2016 14:22:32 -0500
To:    bug-PDF-API2 [...] rt.cpan.org
 
The code produces odd results for a radius $r that's too small. It takes the arcsine of the ratio of the distance between the points divided by twice the given radius. If $r is too small, this is arcsine of a number greater than 1.0, which isn't defined! So definitely, it needs to be fixed. The minimum $r, which needs to be checked for (and possibly fixed) is one-half the distance between the points, giving a semicircle centered at the midway point between the points. The current code leaves the validation of $r to the user.

I think there are two possibilities for this bug. One is that it gives an error message somewhere, possibly disrupting the output, and the second is that it simply silently lengthens the radius $r to one-half the distance between the points, returning a semicircle. Does anyone have a strong opinion one way or the other? How about (in general) error conditions for other calls -- should we silently massage the parameters (where possible) to give minimally acceptable results, and/or give an error message? If an error message is given somewhere (stdout?), should we also "fix up" the call to produce minimally acceptable results (such as the semicircle rather than a funny double arc, in this case)?

In this call, a minimum radius places the arc center midway between the two points, and draws a semicircle. As the radius becomes larger than the minimum, it moves off the line between the two points, and the resulting arc approaches a straight line for very large radii. To make an arc longer than the minimum case (i.e., greater than 180 degrees), you need to set one of the arc() flags ($outer) to 1, and flip ($reverse = 1) the arc.

Apparently, the resulting arc is normally clockwise from point 1 to point 2, at least given the side of the line the arc center moves to.
#
Wed Feb 17 18:38:19 2016 steve [...] deefs.net - Correspondence added

PDF::API2's normal behavior when passed invalid input is undefined.  It'll either blow up (often at a later place than where the input was provided) or accept the invalid input and produce an unusable or unexpected file.

I'd be happy to review a patch that causes bogen() to die immediately if passed invalid input.  That makes more sense to me than trying to guess what the radius should have been.
#
Wed Feb 17 18:38:19 2016 The RT System itself - Status changed from 'new' to 'open'
#
Subject:    [rt.cpan.org #98541]
Date:    Thu, 18 Feb 2016 00:11:27 -0500
To:    bug-PDF-API2 [...] rt.cpan.org
 
We should certainly move towards clearly defining what PDF::API2 does when given invalid input. I can live with an immediate die(). Would it be acceptable to also provide commented-out code to silently "fix" the invalid input and carry on (in this particular case, increase the radius to the minimum)? That way, the user can easily do it that way if they want to. It could be a run-time switch, but that might complicate/slow the code by an unacceptable amount. It would even be possible to have a "hook" for handling invalid inputs: if an invalid input (here, a too-small radius) is seen, call a function. The default behavior for the function would be simply to die, but a user could choose code to do a fixup and return the corrected radius. That way, all this stuff is isolated in a separate routine (same file). For consistency, we would want to do the same for other invalid inputs.

*

Offline Phil

  • Global Moderator
  • Sr. Member
  • *****
  • 356
    • View Profile
Re: RT 98541 - bogen() handling edge cases
« Reply #1: November 14, 2016, 08:27:28 PM »
Fixed in 3.001

  • If the radius is too small, it is silently increased to the minimum allowable size (half the distance between the points).
  • Non-positive radius given is a fatal error.
  • Zero degrees of sweep (not two distinct points) is a fatal error.
  • Clarified POD (documentation) regarding direction of arc draw (which side of points the arc center is on).