On Mon, Sep 29, 2008 at 3:21 PM, Andrew Ross
<[EMAIL PROTECTED]> wrote:
> On Mon, Sep 29, 2008 at 12:00:17PM -0700, Alan Irwin wrote:
>> 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
> extensively internally.
>
> 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.
Would it be possible to add an internal function, not exposed in the
official API, which applies the transform itself, while keeping the
actual plsc->vpw* values unaltered from what the user provides?
Looking through the src/ directory, all of the instances of vpwxmi,
vpwxma, vpwymi, vpwyma are values from plgvpw with the exception of
plwind.c, which is where they are set initially. I am not sure of the
proper naming convention, but perhaps a function named something like
plP_gvpw could be added to the internal PLplot API which adds the
offsets to the viewport limits, but leaves the internal representation
unchanged from what the user provides. Then this internal function
could continue to provide what plgvpw currently does, and plgvpw could
return the original user-provided limits.
I have attached two potential patches to make such a change. They
both add a plP_gvpw function, exported through plplotP.h but not
plplot.h. The first (plP_gvpw-full.patch) replaces all internal
(src/*) calls to plgvpw with plP_gvpw. The second
(plP_gvpw-partial.patch) has the same content, except that the plgvpw
-> plP_gvpw replacement was only done in the label_box function in
plbox.c because that seems to be the only place where it is needed.
Only one of these patches should be applied if this proposal is
accepted.
I am fairly certain that the above patches address all of the vpw*
variables in the PLplot source code. I looked at each occurrence of
vpwmi and they are all results of calls to plgvpw (or, after the above
patch(es), plP_gvpw). The function plgvpw is not used in any of the
examples.
What do you think of this as a compromise? Nothing has to be
propagated to other language bindings, and the plgvpw functionality
becomes (I think) more intuitive.
Hez
--
Hezekiah M. Carty
Graduate Research Assistant
University of Maryland
Department of Atmospheric and Oceanic Science
diff --git a/include/plplot.h b/include/plplot.h
index 18bb60a..17c27ca 100644
--- a/include/plplot.h
+++ b/include/plplot.h
@@ -656,7 +656,6 @@ typedef struct {
#define Free2dGrid plFree2dGrid
#define MinMax2dGrid plMinMax2dGrid
#define plP_gvpd plgvpd
-#define plP_gvpw plgvpw
#define plotsh3d(x,y,z,nx,ny,opt) plsurf3d(x,y,z,nx,ny,opt, NULL, 0)
#endif /* __PLSTUBS_H__ */
diff --git a/include/plplotP.h b/include/plplotP.h
index 1d2590c..08557f1 100644
--- a/include/plplotP.h
+++ b/include/plplotP.h
@@ -620,6 +620,11 @@ cont_store(PLFLT **f, PLINT nx, PLINT ny,
void
cont_clean_store(CONT_LEVEL *ct);
+/* Get the viewport boundaries in world coordinates, expanded slightly */
+
+void
+plP_gvpw(PLFLT *p_xmin, PLFLT *p_xmax, PLFLT *p_ymin, PLFLT *p_ymax);
+
/* Get x-y domain in world coordinates for 3d plots */
void
diff --git a/src/plbox.c b/src/plbox.c
index d6a6a55..4384fc4 100644
--- a/src/plbox.c
+++ b/src/plbox.c
@@ -182,7 +182,7 @@ c_plaxes(PLFLT x0, PLFLT y0,
xtick1 = llx ? 1.0 : xtick;
ytick1 = lly ? 1.0 : ytick;
- plgvpw(&vpwxmin, &vpwxmax, &vpwymin, &vpwymax);
+ plP_gvpw(&vpwxmin, &vpwxmax, &vpwymin, &vpwymax);
/* n.b. large change; vpwxmi always numerically less than vpwxma, and
* similarly for vpwymi */
vpwxmi = (vpwxmax > vpwxmin) ? vpwxmin : vpwxmax;
@@ -1110,7 +1110,7 @@ grid_box(const char *xopt, PLFLT xtick1, PLINT nxsub1,
lhy = plP_stsearch(yopt, 'h');
lly = plP_stsearch(yopt, 'l');
- plgvpw(&vpwxmin, &vpwxmax, &vpwymin, &vpwymax);
+ plP_gvpw(&vpwxmin, &vpwxmax, &vpwymin, &vpwymax);
/* n.b. large change; vpwxmi always numerically less than vpwxma, and
* similarly for vpwymi */
vpwxmi = (vpwxmax > vpwxmin) ? vpwxmin : vpwxmax;
@@ -1221,7 +1221,7 @@ label_box(const char *xopt, PLFLT xtick1, const char *yopt, PLFLT ytick1)
lty = plP_stsearch(yopt, 't');
lvy = plP_stsearch(yopt, 'v');
- plgvpw(&vpwxmin, &vpwxmax, &vpwymin, &vpwymax);
+ plP_gvpw(&vpwxmin, &vpwxmax, &vpwymin, &vpwymax);
/* n.b. large change; vpwxmi always numerically less than vpwxma, and
* similarly for vpwymi */
vpwxmi = (vpwxmax > vpwxmin) ? vpwxmin : vpwxmax;
diff --git a/src/plcore.c b/src/plcore.c
index ed2d10d..a255cea 100644
--- a/src/plcore.c
+++ b/src/plcore.c
@@ -3193,6 +3193,24 @@ c_plgvpw(PLFLT *p_xmin, PLFLT *p_xmax, PLFLT *p_ymin, PLFLT *p_ymax)
*p_ymax = plsc->vpwyma;
}
+/* Get the viewport boundaries in world coordinates, expanded slightly */
+void
+plP_gvpw(PLFLT *p_xmin, PLFLT *p_xmax, PLFLT *p_ymin, PLFLT *p_ymax)
+{
+ PLFLT dx, dy;
+
+ dx = (plsc->vpwxma - plsc->vpwxmi) * 1.0e-5;
+ dy = (plsc->vpwyma - plsc->vpwymi) * 1.0e-5;
+
+ /* The plot window is made slightly larger than requested so that */
+ /* the end limits will be on the graph */
+
+ *p_xmin = plsc->vpwxmi - dx;
+ *p_xmax = plsc->vpwxma + dx;
+ *p_ymin = plsc->vpwymi - dy;
+ *p_ymax = plsc->vpwyma + dy;
+}
+
/*--------------------------------------------------------------------------*\
* These should not be called by the user.
\*--------------------------------------------------------------------------*/
diff --git a/src/plhist.c b/src/plhist.c
index 08363eb..6527690 100644
--- a/src/plhist.c
+++ b/src/plhist.c
@@ -131,7 +131,7 @@ c_plbin(PLINT nbin, PLFLT *x, PLFLT *y, PLINT flags)
}
}
- plgvpw(&vpwxmi, &vpwxma, &vpwymi, &vpwyma);
+ plP_gvpw(&vpwxmi, &vpwxma, &vpwymi, &vpwyma);
if (!(flags & 1)) {
for (i = 0; i < nbin - 1; i++) {
if (!(flags & 4) || (y[i] != vpwymi)) {
diff --git a/src/plwind.c b/src/plwind.c
index df72cc4..796f5bb 100644
--- a/src/plwind.c
+++ b/src/plwind.c
@@ -53,16 +53,10 @@ c_plwind(PLFLT xmin, PLFLT xmax, PLFLT ymin, PLFLT ymax)
ymin--; ymax++;
}
- dx = (xmax - xmin) * 1.0e-5;
- dy = (ymax - ymin) * 1.0e-5;
-
-/* The true plot window is made slightly larger than requested so that */
-/* the end limits will be on the graph */
-
- plsc->vpwxmi = xmin - dx;
- plsc->vpwxma = xmax + dx;
- plsc->vpwymi = ymin - dy;
- plsc->vpwyma = ymax + dy;
+ plsc->vpwxmi = xmin;
+ plsc->vpwxma = xmax;
+ plsc->vpwymi = ymin;
+ plsc->vpwyma = ymax;
/* Compute the scaling between coordinate systems */
diff --git a/include/plplot.h b/include/plplot.h
index 18bb60a..17c27ca 100644
--- a/include/plplot.h
+++ b/include/plplot.h
@@ -656,7 +656,6 @@ typedef struct {
#define Free2dGrid plFree2dGrid
#define MinMax2dGrid plMinMax2dGrid
#define plP_gvpd plgvpd
-#define plP_gvpw plgvpw
#define plotsh3d(x,y,z,nx,ny,opt) plsurf3d(x,y,z,nx,ny,opt, NULL, 0)
#endif /* __PLSTUBS_H__ */
diff --git a/include/plplotP.h b/include/plplotP.h
index 1d2590c..08557f1 100644
--- a/include/plplotP.h
+++ b/include/plplotP.h
@@ -620,6 +620,11 @@ cont_store(PLFLT **f, PLINT nx, PLINT ny,
void
cont_clean_store(CONT_LEVEL *ct);
+/* Get the viewport boundaries in world coordinates, expanded slightly */
+
+void
+plP_gvpw(PLFLT *p_xmin, PLFLT *p_xmax, PLFLT *p_ymin, PLFLT *p_ymax);
+
/* Get x-y domain in world coordinates for 3d plots */
void
diff --git a/src/plbox.c b/src/plbox.c
index d6a6a55..e2620e3 100644
--- a/src/plbox.c
+++ b/src/plbox.c
@@ -1221,7 +1221,7 @@ label_box(const char *xopt, PLFLT xtick1, const char *yopt, PLFLT ytick1)
lty = plP_stsearch(yopt, 't');
lvy = plP_stsearch(yopt, 'v');
- plgvpw(&vpwxmin, &vpwxmax, &vpwymin, &vpwymax);
+ plP_gvpw(&vpwxmin, &vpwxmax, &vpwymin, &vpwymax);
/* n.b. large change; vpwxmi always numerically less than vpwxma, and
* similarly for vpwymi */
vpwxmi = (vpwxmax > vpwxmin) ? vpwxmin : vpwxmax;
diff --git a/src/plcore.c b/src/plcore.c
index ed2d10d..a255cea 100644
--- a/src/plcore.c
+++ b/src/plcore.c
@@ -3193,6 +3193,24 @@ c_plgvpw(PLFLT *p_xmin, PLFLT *p_xmax, PLFLT *p_ymin, PLFLT *p_ymax)
*p_ymax = plsc->vpwyma;
}
+/* Get the viewport boundaries in world coordinates, expanded slightly */
+void
+plP_gvpw(PLFLT *p_xmin, PLFLT *p_xmax, PLFLT *p_ymin, PLFLT *p_ymax)
+{
+ PLFLT dx, dy;
+
+ dx = (plsc->vpwxma - plsc->vpwxmi) * 1.0e-5;
+ dy = (plsc->vpwyma - plsc->vpwymi) * 1.0e-5;
+
+ /* The plot window is made slightly larger than requested so that */
+ /* the end limits will be on the graph */
+
+ *p_xmin = plsc->vpwxmi - dx;
+ *p_xmax = plsc->vpwxma + dx;
+ *p_ymin = plsc->vpwymi - dy;
+ *p_ymax = plsc->vpwyma + dy;
+}
+
/*--------------------------------------------------------------------------*\
* These should not be called by the user.
\*--------------------------------------------------------------------------*/
diff --git a/src/plhist.c b/src/plhist.c
diff --git a/src/plwind.c b/src/plwind.c
index df72cc4..796f5bb 100644
--- a/src/plwind.c
+++ b/src/plwind.c
@@ -53,16 +53,10 @@ c_plwind(PLFLT xmin, PLFLT xmax, PLFLT ymin, PLFLT ymax)
ymin--; ymax++;
}
- dx = (xmax - xmin) * 1.0e-5;
- dy = (ymax - ymin) * 1.0e-5;
-
-/* The true plot window is made slightly larger than requested so that */
-/* the end limits will be on the graph */
-
- plsc->vpwxmi = xmin - dx;
- plsc->vpwxma = xmax + dx;
- plsc->vpwymi = ymin - dy;
- plsc->vpwyma = ymax + dy;
+ plsc->vpwxmi = xmin;
+ plsc->vpwxma = xmax;
+ plsc->vpwymi = ymin;
+ plsc->vpwyma = ymax;
/* Compute the scaling between coordinate systems */
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel
