On Mon, Sep 29, 2008 at 12:00:17PM -0700, Alan Irwin wrote:
> On 2008-09-29 12:47-0400 Hezekiah M. Carty wrote:
> > This is a follow up to the "Viewport Clipping" thread on plplot-general.
> > The attached patch transforms the plsc->vpw* values returned by plgvpw
> > so that they should be the same values as provided by the user.
> > "Should", because there can still be floating point errors in the
> > return values. For example:
> > plenv(-1.0, 1.0, -1.0, 1.0, 0, 0)
> > plgvpw(xmin, xmax, ymin, ymax)
> > will work as expected, returning (-1.0, 1.0, -1.0, 1.0) but:
> > plenv(1.0, 0.0, 1.0, 0.0, 0, 0)
> > plgvpw(xmin, xmax, ymin, ymax)
> > returns with a small error: (1.00000000000000022, 0., 1.00000000000000022, 0.)
> > Greping through the rest of the code in src/, plbox.c and plhist.c
> > both seem potentially affected by this change to plgvpw as they have
> > functions which call plgvpw. At least plbox.c would need to be
> > changed because as it is, plenv will not print the smallest magnitude
> > axis labels in the above two examples.
> > Given the above issues and those raised in the plplot-general thread,
> > perhaps the more prudent approach would be to remove the adjustment
> > factor from plwind.c (the functions c_plwind and probably c_plw3d as
> > well) and find a way to fix any issues in plbox.c and plhist.c. There
> > are probably 3D plotting functions which would have to be adjusted as
> > well if plw3d is changed.
> > As it is, the attached patch is an example of how to invert the
> > plsc->vpw* translations which are currently made, but should probably
> > not be applied to the official PLplot tree until a method of
> > addressing these problems has been picked.
> > Any comments or thoughts?
> I have had some experience with a number of plotting libraries going back
> through at least 1975, and they all have had these little adjustments which
> you remove at your peril. To illustrate that peril, wc gives more than 50
> uses for vpwxmi (set in c_plwind) in the source code in the src directory.
> Presumably some of those are documentation strings, but still that is a lot
> of use. So my feeling is we should internally stick exactly with the
> adjustment in plwind and also return the values exactly as now in plgvpw
> (since plgvpw is also used internally in our code).
> That leaves the question of whether we should design an additional function
> that returns the exact plwind arguments. However, I would argue against that
> because it is a good principle to keep our API to a minimum since adding to
> it means it has to be propagated to all the languages, it has to be
> documented, etc. Note I am generally on the side of expanding our API for
> functions where there is a good chance they will get used, but I do recognize
> there is a cost involved so I want to avoid such expansion for the case
> where the function would be a rarely used part of our API that could be
> done by the user for himself (in this case the user could store and
> retrieve exact plwind arguments for himself).
> Thus, what I would like to do is deal with this issue by improving the
> documentation in a way that encourages the user to save their exact plwind
> arguments rather than trying to compute them from the plgvpw results.
> However, if having considered these arguments others here still feel
> strongly that a documentation improvement is not enough and our API should
> be expanded to deal with this issue instead, then I am would be
> (reluctantly) willing to go along with that.
My original comments to Hez to at least produce the patch were made
without looking to closely at the source. His and Alan's closer look
show that there are lots of complications here with plgvpw being used
I agree with Alan that better documenting the actual behaviour of the
functions is a must. I don't particularly like the idea of adding a new
API function. The two would be so close that it would just confuse the
average user. In most cases the user can keep track of it themselves. I
think what this really suggests is that as an API plgvpw is of limited
use unless the user needs to know the actual rather than requested
viewport window. Perhaps the documentation should also make this clear.
It sounds from the extensive use of the function that "fixing" it to
work without the adjustment might also be a great deal of work for
relatively little gain.