[Plplot-devel] Proposed eop/wait changes have now been committed

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

Hi Jim:

Since you were unable to answer my questions below in a timely manner,
because it is early in the release cycle (so plenty of time to fix any
errors introduced by commits), and because I would like to move on to
other PLplot development; I decided to just go ahead and push these
three commits (for the wxwidgets, tkwin, and wincairo devices).  When
you have time I would still very much appreciate your review of these
three commits (0ba44e87, 8354e461, and ed66632) to make sure my
inferences about what should be changed in the drivers to correspond
to your eop/wait split were correct.

Also, you stated at the time (June 2015) when you made your eop/wait split

"The wxWidgets driver will need to be updated to work correctly because
it uses the plot buffer.  Without an updated, it will need an
extra keypress after a resize event."

I saw no need for such a keypress either before or after my commit
when resizing wxPLViewer results generated by -dev wxwidgets.  So it
is possible my commit is still missing something for the wxwidgets
device case (and even more possible I didn't understand what to look
for there).  Anyhow, please do your own intensive testing of resizing
of the wxPLViewer GUI to see whether you are now satisfied with
how the wxwidgets driver handles the eop/wait split.

Note my own testing revealed the wxwidgets device does have a resize
bug which is that in certain cirmcumstances a resize would always
respond to the mouse position of the _prior_ resize attempt. Those
circumstances were for the second page plotted and all subsequent
pages (even if you used PageUp to back up to the first page again).
The only case where this resize bug did not occur was if the "enter"
or "PageUp" keys had not been pressed to move forward or backward
between pages.  N.B. my tests showed this exact resize bug occurred
both before and after my commit.

Finally, with regard to my wincairo eop/wait implementation it
obviously cannot be tested in the slightest on my Linux platform.  So
I would appreciate you both build and run-time testing that device on
a Windows platform (MinGW-w64/MSYS2?) where native software packages
for the pango/cairo subset of the GTK+ suite of libraries are
available.

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
__________________________

---------- Forwarded message ----------
Date: Thu, 2 Feb 2017 13:50:57 -0800 (PST)
From: Alan W. Irwin <ir...@be...>
To: Jim Dishaw <ji...@di...>, Phil Rosenberg <p.d...@gm...>,
     PLplot development list <Plp...@li...>
Subject: [Plplot-devel] Proposed eop/wait change for -dev wxwidgets

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
__________________________