Hi Alan
That option works for me - if that feature might be in use then we can retain
it.
However I disagree with wrapping plfill in a plstream::plfill method. This is
poor object oriented division of responsibility. The plfill method has nothing
really to do with plstream and if we create that method, then someone somewhere
will use it incorrectly and create a hard to find bug. If you really wanted to
go object oriented then you would create a plfiller class, which would have a
fill virtual method which for the standard class would call plfill. Then you
would pass this into plstream::shade. However, I think all that is unnecessary.
Just using plfill for the callback is simple, effective, transparent and also
conforms to the existing plplot documentation.
I would still however advocate an overload of this function that excludes the
fill parameter altogether. This would leave the existing API, but add a simpler
interface. It's unfortunate that it's location in the parameter list means it
can't really have a default value.
Phil
-----Original Message-----
From: "Alan W. Irwin" <ir...@beluga.phys.uvic.ca>
Sent: 20/10/2014 20:54
To: "phil rosenberg" <philip_rosenb...@yahoo.com>
Cc: "Andrew Ross" <andrewr...@users.sourceforge.net>; "PLplot development list"
<Plplot-devel@lists.sourceforge.net>
Subject: Re: [Plplot-devel] Why is the set_stream() call commented out
forplstream::fill, etc.
On 2014-10-20 10:42-0000 phil rosenberg wrote:
> The fixes as I see them are as follows - getting better in terms of
style (in my opinion), but more difficult as we go.1) Remove the
static and add set_stream calls in plfill etc. In the examples replace
use of plstream::fill callbacks with plfill. Everything now acts as
intended - fundamentally plstream::shade and plshade want a c style
callback so there is no need to pass it a wrapped version of plfill.
@Phil, Andrew:
I don't have the C++ expertise you guys do, but I would nevertheless
like to comment on this topic from the overview perspective.
First of all, just to clear some underbrush out of the way, it is
clear that plfill3 and plgradient are always used directly and never
used as callbacks so making them conform exactly to the rest
of the C++ API (i.e, don't declare them as static and call set_stream
inside) is the obviously correct bug fix to make. I
have made that non-controversial change as of commit e5b1166.
That commit only leaves the special case of fill (used both as a
callback and directly)
to deal with.
I think Phils' first option above (apply the fill3 and gradient
changes to fill for direct use and use something else for the callback
case) is the way to go. After all, Maurice put in this plfill callback
in the plshade argument list back two decades ago
<http://sourceforge.net/p/plplot/plplot/ci/bc27882842d018209e37c7efe0a545a4a318c7e2>,
and some of our users might be taking advantage of it to do something
creative with fills. And it is only after we remove such
functionality (as in Phil's second and 3rd options) that we will
discover whether users are actually using it or not.
I think an improvement to his first option is to remind users of the
correct usage by defining a static plstream::plfill_callback with no
set_stream call inside as an exact wrapper for the C plfill). That is
the general way the other C callbacks (e.g., tr0, tr1, tr2) are
handled.
Note, that although Phil mentioned the C++ and/or C API breakage for
his other options, there is obviously C++ API breakage with this
option as well. For example, those following our old examples for
plshade or plshades using plstream::fill for the callback will run
into trouble as a result of our changes to fill.
Note that regardless of what our final choice is for the two separate
C++ variants of fill that are necessary to take care of direct use and
callback use it is important to recognize the API breakage in the
release notes and tell users exactly how to adjust to the change. (If
the above modified option is chosen, that adjustment would be to
replace all uses of plstream::fill _as a callback_ with
plstream::fill_callback.)
Alan
__________________________
Alan W. Irwin
Astronomical research affiliation with Department of Physics and Astronomy,
University of Victoria (astrowww.phys.uvic.ca).
Programming affiliations with the FreeEOS equation-of-state
implementation for stellar interiors (freeeos.sf.net); the Time
Ephemerides project (timeephem.sf.net); PLplot scientific plotting
software package (plplot.sf.net); the libLASi project
(unifont.org/lasi); the Loads of Linux Links project (loll.sf.net);
and the Linux Brochure Project (lbproject.sf.net).
__________________________
Linux-powered Science
__________________________
------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel
