Hi AndrewJust had a quick look at this as I use the C++ API and I wanted to
make sure it wasn't affecting my code. I hadn't noticed problems, but I think
these three functions have always been preceded by another function which has
called set_stream for my object, so it hasn't affected me so far.
As you say, plstream::fill is being used as a callback function for
plstream::shade and variants. Basically this can't be done, because to work
correctly as a user function plstream::fill cannot be static and must call
set_stream as Alan pointed out.
This problem has probably evolved because of the design of plshade. i think the
fill input should be an enumeration, not a function pointer. This is because
even if other options are added, the fill function is only ever going to be
plplot code (although perhaps I am wrong?). Having this as a function pointer
implies that the user can (or even should) provide their own code for doing the
fill, which isn't really the intention. This implication probably meant that
whoever wrote the C++ code felt obliged to pass a wrapped function rather than
just pass in plfill.
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.2) In addition to above, modify the
plstream::shade functions, removing the fill input altogether. Then always use
plfill in the call to plshade. There is no functionality loss as plfill is the
only possible option anyway and it will remove the potential for user bugs and
make the functions easier to use. It removes the design implications described
above from the C++ interface. If we ever want to reintroduce this option then
C++ overloading makes this trivial. The negative is a C++ API breakage.3)
Change the plshade functions to accept an enumeration of the fill type rather
than a function pointer. Now it is clear to the user that they are simply
making a choice of which plplot coded fill option they want rather than there
being an expectation of them providing their own callback. However this means a
C API breakage, which would need propagation.
One other potential problem is that it might be possible for another plstream
to be used mid render, e.g. if an error callback displays a dialog box in a GUI
program this freezes rendering and may allow rendering of other windows using
other plstreams. Suddenly plsc has been changed mid render and the fill is now
directed to the wrong window. Although plplot is full of such potential
problems, the hiding of the plsetstream call in the C++ binding makes it less
obvious to the user that this can occur. We could fix this with a lock (like
the equivalent of a mutex and mutex locker), which tracks when plstream methods
are in use.
Phil
From: Andrew Ross <andrewr...@users.sourceforge.net>
To: Alan W. Irwin <ir...@beluga.phys.uvic.ca>
Cc: PLplot development list <Plplot-devel@lists.sourceforge.net>
Sent: Friday, 17 October 2014, 22:56
Subject: Re: [Plplot-devel] Why is the set_stream() call commented out for
plstream::fill, etc.
On Thu, Oct 16, 2014 at 04:34:53PM -0700, Alan Irwin wrote:
> On 2014-10-16 20:39+0100 Andrew Ross wrote:
>
> >
> > Alan,
> >
> > It's because these three functions are callback functions which are
> > called from another plplot function. They do not operate directly on
> > a stream.
>
> Hi Andrew:
>
> I may be misinterpreting what you are saying above, but I noticed many
> direct as well as more limited callback uses of these three functions
> in our examples, see below.
>
> >
> > The C++ class maintains a stream variable for each instance of the
> > PLStream class. This means you can have multiple streams (i.e. windows
> > / files) and not have to worry which one plplot is currently using.
> > It's all hidden in the object orientation by calling set_stream
> > each time a plplot API function is called for a particular object.
> >
> > The callback functions don't need to do this since they are being
> > called from another plplot function. If you look in plstream.h
> > you will see these functions are marked static which means there
> > is a single copy of the function shared across all instances which
> > does not have access to the local variables in the class. This
> > means that calling set_stream wouldn't actually work anyway,
> > enforcing this distinction. Uncommenting would generate a compiler
> > error.
>
> You lost me a bit because of my lack of knowledge concerning C++.
> Nevertheless, let me ask a supplementary question. Your explanation
> indicates these are specially treated because they are callback
> functions, but that is not the full story. For example, those
> functions are used directly and not as callbacks in many examples; if
> you look for "fill" in the C++ examples, there are many references to
> pls->fill rather than plstream::fill. I checked a number of those,
> and the pls->fill ones are mostly direct although in example 21 it
> appears that the pls->fill form may also be used as a callback. I
> have no idea whether that example 21 difference in pattern matters,
> but I do wonder whether if one of our users attempted to use fill,
> fill3, or gradient directly in a multistream application similar to
> example 14, would they run into an error because of the lack of
> set_plstream call in the C++ binding? Note our current set of
> examples does not test this case because fill, fill3, and gradient are
> all missing from example 14.
>
> I emphasize again my concerns boil down to questions concerning
> pattern inconsistencies. Thus, you may well have a good C++ answer
> for the pattern inconsistency I noted above for example 21 when it
> appears that fill in direct form (pls->plfill) may be used as a
> callback, and also C++ answers for when fill, fill3, and gradient are
> used directly throughout our examples and also potentially in a
> multistream environment like example 14.
Alan,
OK. On closer examination I think you may be right. Looking at the
plfill code it does seem there is a potential problem. I'll look into
this. Casting my mind back I seem to remember problems with the
callbacks if they weren't static functions, so I may need to think
about how this will work.
Thanks for spotting the problem anyway!
Andrew
------------------------------------------------------------------------------
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
------------------------------------------------------------------------------
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
