[Plplot-devel] Proposed eop/wait change for -dev wxwidgets

[Alan W. I. <ir...@be...>]

On 2017-02-01 17:33-0800 Alan W. Irwin wrote:

> [...] But regardless of these speculations the most
> important point is this current inconsistency between plbuf and
> wxwidgets should be addressed soon since plbuf now distinguishes
> between wait states and eop states and until wxwidgets also follows
> that same model we won't know whether some wxwidgets issue we observe
> [...] is due to this current mismatch

Hi Jim:

>From Phil's ("I'll have a look as soon as I can") response to my post
that included the above remark amongst several others, I suspect he is
pretty tied up with other work.  And there are a whole host of other
issues (including wxPLViewer inefficiency for plots with large numbers
of graphical elements and implementing a native wxwidgets gradient)
that he might want to concentrate on instead.  Therefore, to take some
PLplot load off of him for at least the above issue, I have attempted
to tackle that issue myself (see the attached patch, but more on that
patch later).

When preparing for that I reviewed your wait/eop changes for other
devices, and it looks like in all cases you were attempting to move
all code that is executed for the pls->nopause false case from the
plD_eop_<devicename> function to a newly implemented
plD_wait_<devicename> function (without any check of pls->nopause
because plD_wait* is only called by plP_wait for the pls->nopause
false case which makes such checks in plD_wait_<devicename>
redundant).

If that is a correct summary of what you were attempting to do with
your driver changes I spotted some issues with those changes which are
the following:

1. drivers/cairo.c

You left the wincairo device untouched, and the current
plD_eop_wincairo is nothing but code that is executed in the
pls->nopause false case. It appears to me the solution is to move all
code from plD_eop_wincairo to a new plD_wait_wincairo function while
dropping the redundant check (in that case) for pls->nopause false. Do
you agree?

2. drivers/tkwin.c

Your change copied the pls->nopause false logic in plD_eop_tkwin to
the new routine plD_wait_tkwin (without dropping the redundant check
that pls->nopause was false). It appears to me this should have been a
move of the logic rather than the copy that you did.  Therefore, I
believe this should be followed up by removing the pls->nopause false
logic from plD_eop_tkwin and also removing the redundant check that
pls->nopause is false from plD_wait_tkwin.  Do you agree?

3. drivers/ntk.c

I think you made the right choice, here, i.e., don't touch this driver
at all with regard to wait versus eop, but "for the record" I would
like to remark on this decision here.  There currently is some
pls->nopause false logic used in plD_tidy_ntk (not plD_eop_ntk) to
execute the experimental waitforpage routine.  My guess is this call
to waitforpage should have originally been in plD_eop_ntk, and
therefore now it should be moved to a new plD_wait_ntk routine, but
because of this guesswork and because waitforpage is experimental
(from more than a decade ago by someone who is no longer actively
developing PLplot) your decision to not touch this code at all until
we know a lot more about it (in particular how to even test it!)
is likely correct.

I would now like to move to the topic of my proposed (see my attached
patch) wait/eop changes for the wxwidgets device driver. Here I have
followed the same general idea as your changes for the other drivers.
That is, I implemented a new PLD_wait_wxwidgets routine and moved all
code from PLD_eop_wxwidgets that was executed when pls->nopause was
false to that new routine.  Also, I dropped the redundant check that
pls->nopause was false in the new PLD_wait_wxwidgets routine. As a
result of my change, the only references to pls->nopause that are left
in the -dev wxwidgets code are some logic to exclude the pls->nopause
false case in the (revised) PLD_eop_wxwidgets and some existing logic
in the PLD_tidy_wxwidgets which was already only executed for the
pls->nopause true case.  So I am pretty sure the patch is correct
although it is based on some educated inferences on my part from your
previous eop/wait changes for other drivers rather than deep knowledge
of what is going on!  :-)

My tests of the patch shows it builds without issues, but there are no
changes at run time from it that I can discover. That is, the time
results I got yesterday for example 8 for both with and without the
-np option and the black rather than white color for the first page of
example 16 are the same.  So that is a somewhat disappointing result,
but at least there is no obvious harm that I can see if this patch is
applied.  Of course, that is not enough to justify pushing this patch,
but if in addition you feel confident this patch is correct we
_should_ push it simply to remove this source of uncertainty for
wxwidgets every time we encounter an issue that is associated with
waiting or end of page. Also, please let me know if you think I should
commit and push my suggested changes above for cairo and tkwin or do
you want to handle those yourself?

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
__________________________