plplot-devel

[Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

Attached is a patch series that corrects a NUL termination bug in the plot buffer that I discovered while working on the windows GDI driver.

I apologize that it has not been uncrustified.  Is cygwin required to run the script?

[Plplot-devel] Styling of commits

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

On 2015-06-09 18:18-0400 Jim Dishaw wrote:

> I apologize that [the patch series] has not been uncrustified.

Hi Jim:

This really is no problem, i.e., styling of results is a slight
convenience for the rest of us but not a requirement.  After all, it
is very easy to run the script for those of us here with access to the
required tools which are bash, python, and a build of uncrustify-0.60
(no other version than that one is currently allowed).

However, if you do decide to style your commits, the above requirements
can easily be met on Linux, and I believe Phil found that to be so
on Cygwin.  Furthermore, I assume those same requirements would be easy
to meet on any version of MinGW, and also on Mac OS X.  So there is a
lot of choice for you here.

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
__________________________

Re: [Plplot-devel] Bug fix to plbuf.c

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

On 2015-06-09 18:18-0400 Jim Dishaw wrote:

> Attached is a patch series that corrects a NUL termination bug in
the plot buffer that I discovered while working on the windows GDI
driver.

Hi Jim:

I applied this result to a private branch using "git am", styled this
result (which was very easy to do, see previous comment), amended your
commit with those style changes using "git add" and "git commit
--amend", and then pushed that result using the normal procedure in
README.developers as commit id = 868f790.

Note, I did not test this commit in the slightest, but I assumed you
had done that already.

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
__________________________

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 9, 2015, at 10:39 PM, Alan W. Irwin <ir...@be...> wrote:
> 
> On 2015-06-09 18:18-0400 Jim Dishaw wrote:
> 
>> Attached is a patch series that corrects a NUL termination bug in
> the plot buffer that I discovered while working on the windows GDI
> driver.
> 
> Hi Jim:
> 
> I applied this result to a private branch using "git am", styled this
> result (which was very easy to do, see previous comment), amended your
> commit with those style changes using "git add" and "git commit
> --amend", and then pushed that result using the normal procedure in
> README.developers as commit id = 868f790.
> 
Thanks, I will get uncrustify working on the Windows machine I’m using so I don’t keep inconveniencing you.

> Note, I did not test this commit in the slightest, but I assumed you
> had done that already.

The benefit of developing the the windows GDI driver is that it is exercising code pathways in the plot buffer that have never been tested.  It appears that all the other GUI drivers have shifted to using unicode for strings.  This error affects drivers that use regular strings and rely on the plot buffer, which is a very small set.  I’m still working on the solution to the multiple keypress bug.

Re: [Plplot-devel] Bug fix to plbuf.c

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

On 2015-06-09 23:10-0400 Jim Dishaw wrote:

>
>> On Jun 9, 2015, at 10:39 PM, Alan W. Irwin <ir...@be...> wrote:
>>
>> On 2015-06-09 18:18-0400 Jim Dishaw wrote:
>>
>>> Attached is a patch series that corrects a NUL termination bug in
>> the plot buffer that I discovered while working on the windows GDI
>> driver.
>>
>> Hi Jim:
>>
>> I applied this result to a private branch using "git am", styled this
>> result (which was very easy to do, see previous comment), amended your
>> commit with those style changes using "git add" and "git commit
>> --amend", and then pushed that result using the normal procedure in
>> README.developers as commit id = 868f790.
>>
> Thanks, I will get uncrustify working on the Windows machine I’m using so I don’t keep inconveniencing you.
>
>> Note, I did not test this commit in the slightest, but I assumed you
>> had done that already.
>

> The benefit of developing the the windows GDI driver is that it is
exercising code pathways in the plot buffer that have never been
tested.  It appears that all the other GUI drivers have shifted to
using unicode for strings.  This error affects drivers that use
regular strings and rely on the plot buffer, which is a very small
set.

Hi Jim:

I am sufficiently concerned by your explanation of this fix above that
I am now beginning to wonder a bit if it was necessary.  Of course, I
could be just missing something, but I would appreciate a fuller
explanation of the fix to make sure of its necessity.

Here is some random stuff I know about our method of dealing with
unicode which might help to clarify the discussion.

Input strings for PLplot are all in the UTF-8 representation of
unicode whether representing glyphs that require a multi-byte UTF-8
representation or glyphs that require only a single-byte (ascii) but
still UTF-8 representation of glyphs. In all cases these input strings
are a series of bytes which are null-terminated. So I really don't see
how you are getting null-termination problems for some devices and not
others.  Also, the xwin device driver (which is available on Cygwin,
Mac OS X, and Linux) is an example of a driver that uses regular
strings and relies on the plot buffer.  So this driver should be an
excellent test case for us of plbuf changes that you do for the
regular strings case.

How should the latest change of yours affect the xwin device driver
results?

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
__________________________

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 10, 2015, at 3:57 AM, Alan W. Irwin <ir...@be...> wrote:
> 
>> On 2015-06-09 23:10-0400 Jim Dishaw wrote:
>> 
>> 
>>> On Jun 9, 2015, at 10:39 PM, Alan W. Irwin <ir...@be...> wrote:
>>> 
>>> On 2015-06-09 18:18-0400 Jim Dishaw wrote:
>>> 
>>>> Attached is a patch series that corrects a NUL termination bug in
>>> the plot buffer that I discovered while working on the windows GDI
>>> driver.
>>> 
>>> Hi Jim:
>>> 
>>> I applied this result to a private branch using "git am", styled this
>>> result (which was very easy to do, see previous comment), amended your
>>> commit with those style changes using "git add" and "git commit
>>> --amend", and then pushed that result using the normal procedure in
>>> README.developers as commit id = 868f790.
>> Thanks, I will get uncrustify working on the Windows machine I’m using so I don’t keep inconveniencing you.
>> 
>>> Note, I did not test this commit in the slightest, but I assumed you
>>> had done that already.
> 
>> The benefit of developing the the windows GDI driver is that it is
> exercising code pathways in the plot buffer that have never been
> tested.  It appears that all the other GUI drivers have shifted to
> using unicode for strings.  This error affects drivers that use
> regular strings and rely on the plot buffer, which is a very small
> set.
> 
> Hi Jim:
> 
> I am sufficiently concerned by your explanation of this fix above that
> I am now beginning to wonder a bit if it was necessary.  Of course, I
> could be just missing something, but I would appreciate a fuller
> explanation of the fix to make sure of its necessity.
> 

The fix is still necessary to replay the plot buffer with a device that uses char strings vice the Unicode string. In EscText there are two different ways of storing strings. 

> Here is some random stuff I know about our method of dealing with
> unicode which might help to clarify the discussion.
> 
> Input strings for PLplot are all in the UTF-8 representation of
> unicode whether representing glyphs that require a multi-byte UTF-8
> representation or glyphs that require only a single-byte (ascii) but
> still UTF-8 representation of glyphs. In all cases these input strings
> are a series of bytes which are null-terminated. So I really don't see
> how you are getting null-termination problems for some devices and not
> others.  Also, the xwin device driver (which is available on Cygwin,
> Mac OS X, and Linux) is an example of a driver that uses regular
> strings and relies on the plot buffer.  So this driver should be an
> excellent test case for us of plbuf changes that you do for the
> regular strings case.
> 
> How should the latest change of yours affect the xwin device driver
> results?
> 

Without the patch text strings could show random characters on redraw events. With this patch all the strings display correctly on resize events. 

> 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
> __________________________

Re: [Plplot-devel] Bug fix to plbuf.c

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

On 2015-06-10 19:58-0400 Jim Dishaw wrote:

>> On Jun 10, 2015, at 3:57 AM, Alan W. Irwin <ir...@be...> wrote:

>> I am sufficiently concerned by your explanation of this fix above that
>> I am now beginning to wonder a bit if it was necessary.  Of course, I
>> could be just missing something, but I would appreciate a fuller
>> explanation of the fix to make sure of its necessity.
>>
>

> The fix is still necessary to replay the plot buffer with a device
that uses char strings vice the Unicode string. In EscText there are
two different ways of storing strings.

Hi Jim:

Thanks for that additional explanation. To be clear about that, input
strings in PLplot are a null-terminated series of bytes that
unicode-unaware devices use directly, and those are always also
transformed to a known-length array of 32-bit integers that
unicode-aware devices use.  And I guess I can see why you want to
store both results in the buffer rather than recreating the
known-length array from the null-terminated series of bytes for each
resizing event. Anyhow, if previously you were ignoring the NULL
termination on the representation using a series of bytes, that is
indeed a bug, and I am glad you fixed it.

> Without the patch text strings could show random characters on
redraw events. With this patch all the strings display correctly on
resize events.

I never noticed character problems like that when resizing -dev xwin, but my eye
may have just missed it.

Just to confirm that I just now played a lot with resizing of

examples/c/x01c -dev xwin

for 5.10.0, 5.11.0, and git master tip (including your recent commit),
and I found no specific string issues for any of those cases.  So it
appears hard (at least with this example) to demonstrate a rendering
bug due to this issue that you just fixed, but your fix seems logical
to me (now) and certainly does not introduce any bad string rendering
that I can spot.

However, when doing those checks I did notice other resizing issues
that do need to be addressed.

1. As a resize is occurring (typically by dragging one edge or one
corner of the GUI to make the whole thing smaller or larger) sometimes
the displayed GUI is a scaled plot and sometimes it is black (which is
OK).  But on some occasions when I stop resizing (by releasing the
mouse button) the result remains black for 5.11.0 and the master tip
version (which is not OK).  But 5.10.0 is fine in this regard. So this
issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
and probably due to the flurry of changes you and Phil made in plbuf
before we released 5.11.0.

Please verify the good result for 5.10.0 and bad result for 5.11.0,
git bisect to figure out what commit has caused the issue, and then
make the appropriate plbuf fix to get rid of this regression.

2. For certain rare patterns of resizing the displayed plot results
are not scaled properly to fit into the resized GUI so that the plot
is larger than the resized GUI and clipped at its edges.  Changing the
resizing slightly immediately fixes the issue completely. The badly
scaled result is rare so you sometimes have to play around with
resizing a minute or so until you see it.  This effect is seen in all
versions of PLplot that I tested. So this issue is a long-standing bug
in -dev xwin or else the plbuf code rather than a recently introduced
regression. To help pin this long-standing bug down further, please
check if it also occurs for either/both wingcc or your new GDI device.
If so then this is probably a long-standing plbuf bug you will want to
address with much higher priority (since it likely affects all
interactive results that are displayed with the help of plbuf) than a
bug in just -dev xwin.

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
__________________________

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 10, 2015, at 11:30 PM, Alan W. Irwin <ir...@be...> wrote:
> 
> On 2015-06-10 19:58-0400 Jim Dishaw wrote:
> 
>>> On Jun 10, 2015, at 3:57 AM, Alan W. Irwin <ir...@be...> wrote:
> 
>>> I am sufficiently concerned by your explanation of this fix above that
>>> I am now beginning to wonder a bit if it was necessary.  Of course, I
>>> could be just missing something, but I would appreciate a fuller
>>> explanation of the fix to make sure of its necessity.
>>> 
>> 
> 
>> The fix is still necessary to replay the plot buffer with a device
> that uses char strings vice the Unicode string. In EscText there are
> two different ways of storing strings.
> 
> Hi Jim:
> 
> Thanks for that additional explanation. To be clear about that, input
> strings in PLplot are a null-terminated series of bytes that
> unicode-unaware devices use directly, and those are always also
> transformed to a known-length array of 32-bit integers that
> unicode-aware devices use.  And I guess I can see why you want to
> store both results in the buffer rather than recreating the
> known-length array from the null-terminated series of bytes for each
> resizing event. Anyhow, if previously you were ignoring the NULL
> termination on the representation using a series of bytes, that is
> indeed a bug, and I am glad you fixed it.
> 
>> Without the patch text strings could show random characters on
> redraw events. With this patch all the strings display correctly on
> resize events.
> 
> I never noticed character problems like that when resizing -dev xwin, but my eye
> may have just missed it.
> 

I never was able to reproduce the problem reliably. It was working fine and then as I added features to the text processing in wingdi, the problem would manifest on some of the examples.  

> Just to confirm that I just now played a lot with resizing of
> 
> examples/c/x01c -dev xwin
> 
> for 5.10.0, 5.11.0, and git master tip (including your recent commit),
> and I found no specific string issues for any of those cases.  So it
> appears hard (at least with this example) to demonstrate a rendering
> bug due to this issue that you just fixed, but your fix seems logical
> to me (now) and certainly does not introduce any bad string rendering
> that I can spot.
> 
> However, when doing those checks I did notice other resizing issues
> that do need to be addressed.
> 
> 1. As a resize is occurring (typically by dragging one edge or one
> corner of the GUI to make the whole thing smaller or larger) sometimes
> the displayed GUI is a scaled plot and sometimes it is black (which is
> OK).  But on some occasions when I stop resizing (by releasing the
> mouse button) the result remains black for 5.11.0 and the master tip
> version (which is not OK).  But 5.10.0 is fine in this regard. So this
> issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
> and probably due to the flurry of changes you and Phil made in plbuf
> before we released 5.11.0.

> 
> Please verify the good result for 5.10.0 and bad result for 5.11.0,
> git bisect to figure out what commit has caused the issue, and then
> make the appropriate plbuf fix to get rid of this regression.
> 

I think this problem is the plP_eop() that was inserted in the plRemakePlot() to make sure the BOP would perform as expected.  For the GUI drivers this will trigger another wait for input, which ends up blocking redraw messages.  I will verify by using git bisect.

> 2. For certain rare patterns of resizing the displayed plot results
> are not scaled properly to fit into the resized GUI so that the plot
> is larger than the resized GUI and clipped at its edges.  Changing the
> resizing slightly immediately fixes the issue completely. The badly
> scaled result is rare so you sometimes have to play around with
> resizing a minute or so until you see it.  This effect is seen in all
> versions of PLplot that I tested. So this issue is a long-standing bug
> in -dev xwin or else the plbuf code rather than a recently introduced
> regression. To help pin this long-standing bug down further, please
> check if it also occurs for either/both wingcc or your new GDI device.
> If so then this is probably a long-standing plbuf bug you will want to
> address with much higher priority (since it likely affects all
> interactive results that are displayed with the help of plbuf) than a
> bug in just -dev xwin.
> 

The problem most likely is in the driver because plbuf is agnostic on the device dimension and scaling. I have a couple of guesses on the cause of this bug.  I will investigate and see if I can patch. 

It will be good to look at xwin because I want to add the ability to open and save plot metafiles for the new version of plrender.  I need to add menus and callback functions to the GUI drivers that plrender can use.  My thought is to have a driver option that would enable that feature.

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 11, 2015, at 12:29 AM, Jim Dishaw <ji...@di...> wrote:
> 
>> 
>> On Jun 10, 2015, at 11:30 PM, Alan W. Irwin <ir...@be...> wrote:
 <snip>
>> Just to confirm that I just now played a lot with resizing of
>> 
>> examples/c/x01c -dev xwin
>> 
>> for 5.10.0, 5.11.0, and git master tip (including your recent commit),
>> and I found no specific string issues for any of those cases.  So it
>> appears hard (at least with this example) to demonstrate a rendering
>> bug due to this issue that you just fixed, but your fix seems logical
>> to me (now) and certainly does not introduce any bad string rendering
>> that I can spot.
>> 
>> However, when doing those checks I did notice other resizing issues
>> that do need to be addressed.
>> 
>> 1. As a resize is occurring (typically by dragging one edge or one
>> corner of the GUI to make the whole thing smaller or larger) sometimes
>> the displayed GUI is a scaled plot and sometimes it is black (which is
>> OK).  But on some occasions when I stop resizing (by releasing the
>> mouse button) the result remains black for 5.11.0 and the master tip
>> version (which is not OK).  But 5.10.0 is fine in this regard. So this
>> issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
>> and probably due to the flurry of changes you and Phil made in plbuf
>> before we released 5.11.0.
> 
>> 
>> Please verify the good result for 5.10.0 and bad result for 5.11.0,
>> git bisect to figure out what commit has caused the issue, and then
>> make the appropriate plbuf fix to get rid of this regression.
>> 
> 
> I think this problem is the plP_eop() that was inserted in the plRemakePlot() to make sure the BOP would perform as expected.  For the GUI drivers this will trigger another wait for input, which ends up blocking redraw messages.  I will verify by using git bisect.

I ran git bisect and the regression was introduced in the commit 1e402417c1f3e87c391fe428f936153c2a10e8cc

Author: Phil Rosenberg <p.d...@gm...>
Date:   Fri Feb 27 17:12:03 2015 +0000

    Fixed bug in rdbuf_di.
    
    Save cursubpage on replot
    call plP_eop() on replot which ensures that the first plP_bop() call actually does something.

If I comment out the plP_eop() that was added to plbuf.c in that commit, the xwin driver does not exhibit the problematic behavior.  I will investigate further on working on a patch.  I know why it is causing a problem, but it isn’t obvious to me how it gets to that point.

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 13, 2015, at 12:03 PM, Jim Dishaw <ji...@di...> wrote:
> 
> 
>> On Jun 11, 2015, at 12:29 AM, Jim Dishaw <ji...@di... <mailto:ji...@di...>> wrote:
>> 
>>> 
>>> On Jun 10, 2015, at 11:30 PM, Alan W. Irwin <ir...@be... <mailto:ir...@be...>> wrote:
>  <snip>
>>> Just to confirm that I just now played a lot with resizing of
>>> 
>>> examples/c/x01c -dev xwin
>>> 
>>> for 5.10.0, 5.11.0, and git master tip (including your recent commit),
>>> and I found no specific string issues for any of those cases.  So it
>>> appears hard (at least with this example) to demonstrate a rendering
>>> bug due to this issue that you just fixed, but your fix seems logical
>>> to me (now) and certainly does not introduce any bad string rendering
>>> that I can spot.
>>> 
>>> However, when doing those checks I did notice other resizing issues
>>> that do need to be addressed.
>>> 
>>> 1. As a resize is occurring (typically by dragging one edge or one
>>> corner of the GUI to make the whole thing smaller or larger) sometimes
>>> the displayed GUI is a scaled plot and sometimes it is black (which is
>>> OK).  But on some occasions when I stop resizing (by releasing the
>>> mouse button) the result remains black for 5.11.0 and the master tip
>>> version (which is not OK).  But 5.10.0 is fine in this regard. So this
>>> issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
>>> and probably due to the flurry of changes you and Phil made in plbuf
>>> before we released 5.11.0.
>> 
>>> 
>>> Please verify the good result for 5.10.0 and bad result for 5.11.0,
>>> git bisect to figure out what commit has caused the issue, and then
>>> make the appropriate plbuf fix to get rid of this regression.
>>> 
>> 
>> I think this problem is the plP_eop() that was inserted in the plRemakePlot() to make sure the BOP would perform as expected.  For the GUI drivers this will trigger another wait for input, which ends up blocking redraw messages.  I will verify by using git bisect.
> 
> I ran git bisect and the regression was introduced in the commit 1e402417c1f3e87c391fe428f936153c2a10e8cc
> 
> Author: Phil Rosenberg <p.d...@gm... <mailto:p.d...@gm...>>
> Date:   Fri Feb 27 17:12:03 2015 +0000
> 
>     Fixed bug in rdbuf_di.
>     
>     Save cursubpage on replot
>     call plP_eop() on replot which ensures that the first plP_bop() call actually does something.
> 
> If I comment out the plP_eop() that was added to plbuf.c in that commit, the xwin driver does not exhibit the problematic behavior.  I will investigate further on working on a patch.  I know why it is causing a problem, but it isn’t obvious to me how it gets to that point.
> 

I found the cause of the problem and the plP_eop() is not acting in a way that neither Phil nor I expected.  Phil put the plP_eop() in to make sure that the BOP will do something and I thought that will always to lead to the keypress bug.  In reality the plP_eop() does something only half the time.

1) Plot starts
2) At BOP, the page_status is set to AT_BOP
3) While drawing, the page_status is set to DRAWING
4) At EOP, the page_status is set to AT_EOP  (the EOP is not put into the buffer)
5) The driver enters into a loop waiting for messages
6) Driver receives a resize message
7) plRemakePlot is called
8) plRemakPlot() calls plP_eop()
9) plP_eop() does nothing because it exits immediately because the page_status is AT_EOP
10) The plot buffer is replayed
11) At BOP, the page_status is set to AT_BOP
12) While drawing, the page_status is set to DRAWING
13) No EOP is generated because it is not in the buffer, thus the page_status is unchanged
14) Control returns the message loop
15) Another resize message is received
16) plRemakePlot is called
17) plRemakePlot() calls plP_eop()
18) plP_eop() does not exit immediately this time because page_state is DRAWING
19) plP_eop() calls the driver EOP handler
20) The driver enters into a new loop waiting for messages
21) User has to do an action (e.g. keypress) to exit the new message loop.  This blocks many messages because the windowing system still considers the window to be processing a resize.
22) Once the new message loop exits, the driver EOP handler exits.
23) Goto step 10.

Just putting the EOP into the plot buffer won’t fix the problem because a new message loop will be called, which will block messages.  

I agree with Phil’s point about the need to call plP_eop().  Some drivers may need the EOP handler to work correctly (e.g. double buffering). The way the code is structured, the plP_eop() is only doing something 50% of the time. 

I’m not sure the best way to fix this.  I think the interactive GUI drivers might need to be modified to check if they are already waiting.  One thing that makes this tricky is the strip chart function.  As it is currently written, it does not generate an EOP until the end.  Thus, it cannot be resized while plotting.  Plus, when resized, the entire strip chart is replayed (though I see Phil has made some commits so wxWidgets might be smarter).

I’m going to experiment on some different approaches and see what works best.

Re: [Plplot-devel] Bug fix to plbuf.c

[Phil R. <p.d...@gm...>]

Hmmm
Then hmmmm some more
Followed by an ummm or two.

As you say Jim, clearly things are more complicated than I realised. I'm not sure what the requirements really are here now. Now you have brought this up I can imagine the issues associated with entering the message loop after an eop.

In terms of dealing with it, I'm not at my computer right now, but I think probably a bop_forced function which forces a bop irrespective of where we are in the page cycle would work. But I'm not sure if that might have some potential subtle effects elsewhere so it is a bit of a hack really. Perhaps instead it would be better to assess what is being done in a bop and ensure those actions end up in the buffer so a bop event is not required.

Phil

-----Original Message-----
From: "Jim Dishaw" <ji...@di...>
Sent: ‎13/‎06/‎2015 17:55
To: "Alan W. Irwin" <ir...@be...>
Cc: "PLplot development list" <Plp...@li...>
Subject: Re: [Plplot-devel] Bug fix to plbuf.c



On Jun 13, 2015, at 12:03 PM, Jim Dishaw <ji...@di...> wrote:




On Jun 11, 2015, at 12:29 AM, Jim Dishaw <ji...@di...> wrote:



On Jun 10, 2015, at 11:30 PM, Alan W. Irwin <ir...@be...> wrote:
 <snip>

Just to confirm that I just now played a lot with resizing of

examples/c/x01c -dev xwin

for 5.10.0, 5.11.0, and git master tip (including your recent commit),
and I found no specific string issues for any of those cases.  So it
appears hard (at least with this example) to demonstrate a rendering
bug due to this issue that you just fixed, but your fix seems logical
to me (now) and certainly does not introduce any bad string rendering
that I can spot.

However, when doing those checks I did notice other resizing issues
that do need to be addressed.

1. As a resize is occurring (typically by dragging one edge or one
corner of the GUI to make the whole thing smaller or larger) sometimes
the displayed GUI is a scaled plot and sometimes it is black (which is
OK).  But on some occasions when I stop resizing (by releasing the
mouse button) the result remains black for 5.11.0 and the master tip
version (which is not OK).  But 5.10.0 is fine in this regard. So this
issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
and probably due to the flurry of changes you and Phil made in plbuf
before we released 5.11.0.




Please verify the good result for 5.10.0 and bad result for 5.11.0,
git bisect to figure out what commit has caused the issue, and then
make the appropriate plbuf fix to get rid of this regression.



I think this problem is the plP_eop() that was inserted in the plRemakePlot() to make sure the BOP would perform as expected.  For the GUI drivers this will trigger another wait for input, which ends up blocking redraw messages.  I will verify by using git bisect.



I ran git bisect and the regression was introduced in the commit 1e402417c1f3e87c391fe428f936153c2a10e8cc


Author: Phil Rosenberg <p.d...@gm...>
Date:   Fri Feb 27 17:12:03 2015 +0000


    Fixed bug in rdbuf_di.
    
    Save cursubpage on replot
    call plP_eop() on replot which ensures that the first plP_bop() call actually does something.


If I comment out the plP_eop() that was added to plbuf.c in that commit, the xwin driver does not exhibit the problematic behavior.  I will investigate further on working on a patch.  I know why it is causing a problem, but it isn’t obvious to me how it gets to that point.




I found the cause of the problem and the plP_eop() is not acting in a way that neither Phil nor I expected.  Phil put the plP_eop() in to make sure that the BOP will do something and I thought that will always to lead to the keypress bug.  In reality the plP_eop() does something only half the time.


1) Plot starts
2) At BOP, the page_status is set to AT_BOP
3) While drawing, the page_status is set to DRAWING
4) At EOP, the page_status is set to AT_EOP  (the EOP is not put into the buffer)
5) The driver enters into a loop waiting for messages
6) Driver receives a resize message
7) plRemakePlot is called
8) plRemakPlot() calls plP_eop()
9) plP_eop() does nothing because it exits immediately because the page_status is AT_EOP
10) The plot buffer is replayed
11) At BOP, the page_status is set to AT_BOP
12) While drawing, the page_status is set to DRAWING
13) No EOP is generated because it is not in the buffer, thus the page_status is unchanged
14) Control returns the message loop
15) Another resize message is received
16) plRemakePlot is called
17) plRemakePlot() calls plP_eop()
18) plP_eop() does not exit immediately this time because page_state is DRAWING
19) plP_eop() calls the driver EOP handler
20) The driver enters into a new loop waiting for messages
21) User has to do an action (e.g. keypress) to exit the new message loop.  This blocks many messages because the windowing system still considers the window to be processing a resize.
22) Once the new message loop exits, the driver EOP handler exits.
23) Goto step 10.


Just putting the EOP into the plot buffer won’t fix the problem because a new message loop will be called, which will block messages.  


I agree with Phil’s point about the need to call plP_eop().  Some drivers may need the EOP handler to work correctly (e.g. double buffering). The way the code is structured, the plP_eop() is only doing something 50% of the time. 


I’m not sure the best way to fix this.  I think the interactive GUI drivers might need to be modified to check if they are already waiting.  One thing that makes this tricky is the strip chart function.  As it is currently written, it does not generate an EOP until the end.  Thus, it cannot be resized while plotting.  Plus, when resized, the entire strip chart is replayed (though I see Phil has made some commits so wxWidgets might be smarter).


I’m going to experiment on some different approaches and see what works best.

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 13, 2015, at 1:13 PM, Phil Rosenberg <p.d...@gm...> wrote:
> 
> Hmmm
> Then hmmmm some more
> Followed by an ummm or two.
> 
> As you say Jim, clearly things are more complicated than I realised. I'm not sure what the requirements really are here now. Now you have brought this up I can imagine the issues associated with entering the message loop after an eop.
> 
> In terms of dealing with it, I'm not at my computer right now, but I think probably a bop_forced function which forces a bop irrespective of where we are in the page cycle would work. But I'm not sure if that might have some potential subtle effects elsewhere so it is a bit of a hack really. Perhaps instead it would be better to assess what is being done in a bop and ensure those actions end up in the buffer so a bop event is not required.
> 

I think that is a good approach for reasons beyond this issue, but a BOP and EOP of some type is still required to support the drivers adequately (e.g double buffering, printing from the windows API).  

The solution I'm going to test is one where the driver does not enter into a wait for input state on a redraw event from the callback. 

> Phil
> From: Jim Dishaw
> Sent: ‎13/‎06/‎2015 17:55
> To: Alan W. Irwin
> Cc: PLplot development list
> Subject: Re: [Plplot-devel] Bug fix to plbuf.c
> 
> 
>>> On Jun 13, 2015, at 12:03 PM, Jim Dishaw <ji...@di...> wrote:
>>> 
>>> 
>>>> On Jun 11, 2015, at 12:29 AM, Jim Dishaw <ji...@di...> wrote:
>>>> 
>>>> 
>>>> On Jun 10, 2015, at 11:30 PM, Alan W. Irwin <ir...@be...> wrote:
>>  <snip>
>>>> Just to confirm that I just now played a lot with resizing of
>>>> 
>>>> examples/c/x01c -dev xwin
>>>> 
>>>> for 5.10.0, 5.11.0, and git master tip (including your recent commit),
>>>> and I found no specific string issues for any of those cases.  So it
>>>> appears hard (at least with this example) to demonstrate a rendering
>>>> bug due to this issue that you just fixed, but your fix seems logical
>>>> to me (now) and certainly does not introduce any bad string rendering
>>>> that I can spot.
>>>> 
>>>> However, when doing those checks I did notice other resizing issues
>>>> that do need to be addressed.
>>>> 
>>>> 1. As a resize is occurring (typically by dragging one edge or one
>>>> corner of the GUI to make the whole thing smaller or larger) sometimes
>>>> the displayed GUI is a scaled plot and sometimes it is black (which is
>>>> OK).  But on some occasions when I stop resizing (by releasing the
>>>> mouse button) the result remains black for 5.11.0 and the master tip
>>>> version (which is not OK).  But 5.10.0 is fine in this regard. So this
>>>> issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
>>>> and probably due to the flurry of changes you and Phil made in plbuf
>>>> before we released 5.11.0.
>>> 
>>>> 
>>>> Please verify the good result for 5.10.0 and bad result for 5.11.0,
>>>> git bisect to figure out what commit has caused the issue, and then
>>>> make the appropriate plbuf fix to get rid of this regression.
>>> 
>>> I think this problem is the plP_eop() that was inserted in the plRemakePlot() to make sure the BOP would perform as expected.  For the GUI drivers this will trigger another wait for input, which ends up blocking redraw messages.  I will verify by using git bisect.
>> 
>> I ran git bisect and the regression was introduced in the commit 1e402417c1f3e87c391fe428f936153c2a10e8cc
>> 
>> Author: Phil Rosenberg <p.d...@gm...>
>> Date:   Fri Feb 27 17:12:03 2015 +0000
>> 
>>     Fixed bug in rdbuf_di.
>>     
>>     Save cursubpage on replot
>>     call plP_eop() on replot which ensures that the first plP_bop() call actually does something.
>> 
>> If I comment out the plP_eop() that was added to plbuf.c in that commit, the xwin driver does not exhibit the problematic behavior.  I will investigate further on working on a patch.  I know why it is causing a problem, but it isn’t obvious to me how it gets to that point.
> 
> I found the cause of the problem and the plP_eop() is not acting in a way that neither Phil nor I expected.  Phil put the plP_eop() in to make sure that the BOP will do something and I thought that will always to lead to the keypress bug.  In reality the plP_eop() does something only half the time.
> 
> 1) Plot starts
> 2) At BOP, the page_status is set to AT_BOP
> 3) While drawing, the page_status is set to DRAWING
> 4) At EOP, the page_status is set to AT_EOP  (the EOP is not put into the buffer)
> 5) The driver enters into a loop waiting for messages
> 6) Driver receives a resize message
> 7) plRemakePlot is called
> 8) plRemakPlot() calls plP_eop()
> 9) plP_eop() does nothing because it exits immediately because the page_status is AT_EOP
> 10) The plot buffer is replayed
> 11) At BOP, the page_status is set to AT_BOP
> 12) While drawing, the page_status is set to DRAWING
> 13) No EOP is generated because it is not in the buffer, thus the page_status is unchanged
> 14) Control returns the message loop
> 15) Another resize message is received
> 16) plRemakePlot is called
> 17) plRemakePlot() calls plP_eop()
> 18) plP_eop() does not exit immediately this time because page_state is DRAWING
> 19) plP_eop() calls the driver EOP handler
> 20) The driver enters into a new loop waiting for messages
> 21) User has to do an action (e.g. keypress) to exit the new message loop.  This blocks many messages because the windowing system still considers the window to be processing a resize.
> 22) Once the new message loop exits, the driver EOP handler exits.
> 23) Goto step 10.
> 
> Just putting the EOP into the plot buffer won’t fix the problem because a new message loop will be called, which will block messages.  
> 
> I agree with Phil’s point about the need to call plP_eop().  Some drivers may need the EOP handler to work correctly (e.g. double buffering). The way the code is structured, the plP_eop() is only doing something 50% of the time. 
> 
> I’m not sure the best way to fix this.  I think the interactive GUI drivers might need to be modified to check if they are already waiting.  One thing that makes this tricky is the strip chart function.  As it is currently written, it does not generate an EOP until the end.  Thus, it cannot be resized while plotting.  Plus, when resized, the entire strip chart is replayed (though I see Phil has made some commits so wxWidgets might be smarter).
> 
> I’m going to experiment on some different approaches and see what works best.
> 

Re: [Plplot-devel] Bug fix to plbuf.c

[Phil R. <p.d...@gm...>]

Okay, well if you need any help or testing let me know.

Something that I haven't seen up to now (but maybe I never looked hard enough) is a spec sheet for how to write a driver, I.e. What events should a buffer deal with and in what way. Given the effort I've just been through with rewriting wxWidgets driver and what you are going through with the windows driver, perhaps we should write it, and then we can refer to it and if we wish to tidy the core-driver interface in the future it will be a useful reference.

Phil

-----Original Message-----
From: "Jim Dishaw" <ji...@di...>
Sent: ‎13/‎06/‎2015 19:11
To: "Phil Rosenberg" <p.d...@gm...>
Cc: "Alan W. Irwin" <ir...@be...>; "PLplot development list" <Plp...@li...>
Subject: Re: [Plplot-devel] Bug fix to plbuf.c



On Jun 13, 2015, at 1:13 PM, Phil Rosenberg <p.d...@gm...> wrote:


Hmmm
Then hmmmm some more
Followed by an ummm or two.

As you say Jim, clearly things are more complicated than I realised. I'm not sure what the requirements really are here now. Now you have brought this up I can imagine the issues associated with entering the message loop after an eop.

In terms of dealing with it, I'm not at my computer right now, but I think probably a bop_forced function which forces a bop irrespective of where we are in the page cycle would work. But I'm not sure if that might have some potential subtle effects elsewhere so it is a bit of a hack really. Perhaps instead it would be better to assess what is being done in a bop and ensure those actions end up in the buffer so a bop event is not required.




I think that is a good approach for reasons beyond this issue, but a BOP and EOP of some type is still required to support the drivers adequately (e.g double buffering, printing from the windows API).  


The solution I'm going to test is one where the driver does not enter into a wait for input state on a redraw event from the callback. 


Phil


From: Jim Dishaw
Sent: ‎13/‎06/‎2015 17:55
To: Alan W. Irwin
Cc: PLplot development list
Subject: Re: [Plplot-devel] Bug fix to plbuf.c




On Jun 13, 2015, at 12:03 PM, Jim Dishaw <ji...@di...> wrote:




On Jun 11, 2015, at 12:29 AM, Jim Dishaw <ji...@di...> wrote:



On Jun 10, 2015, at 11:30 PM, Alan W. Irwin <ir...@be...> wrote:
 <snip>

Just to confirm that I just now played a lot with resizing of

examples/c/x01c -dev xwin

for 5.10.0, 5.11.0, and git master tip (including your recent commit),
and I found no specific string issues for any of those cases.  So it
appears hard (at least with this example) to demonstrate a rendering
bug due to this issue that you just fixed, but your fix seems logical
to me (now) and certainly does not introduce any bad string rendering
that I can spot.

However, when doing those checks I did notice other resizing issues
that do need to be addressed.

1. As a resize is occurring (typically by dragging one edge or one
corner of the GUI to make the whole thing smaller or larger) sometimes
the displayed GUI is a scaled plot and sometimes it is black (which is
OK).  But on some occasions when I stop resizing (by releasing the
mouse button) the result remains black for 5.11.0 and the master tip
version (which is not OK).  But 5.10.0 is fine in this regard. So this
issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
and probably due to the flurry of changes you and Phil made in plbuf
before we released 5.11.0.




Please verify the good result for 5.10.0 and bad result for 5.11.0,
git bisect to figure out what commit has caused the issue, and then
make the appropriate plbuf fix to get rid of this regression.



I think this problem is the plP_eop() that was inserted in the plRemakePlot() to make sure the BOP would perform as expected.  For the GUI drivers this will trigger another wait for input, which ends up blocking redraw messages.  I will verify by using git bisect.



I ran git bisect and the regression was introduced in the commit 1e402417c1f3e87c391fe428f936153c2a10e8cc


Author: Phil Rosenberg <p.d...@gm...>
Date:   Fri Feb 27 17:12:03 2015 +0000


    Fixed bug in rdbuf_di.
    
    Save cursubpage on replot
    call plP_eop() on replot which ensures that the first plP_bop() call actually does something.


If I comment out the plP_eop() that was added to plbuf.c in that commit, the xwin driver does not exhibit the problematic behavior.  I will investigate further on working on a patch.  I know why it is causing a problem, but it isn’t obvious to me how it gets to that point.




I found the cause of the problem and the plP_eop() is not acting in a way that neither Phil nor I expected.  Phil put the plP_eop() in to make sure that the BOP will do something and I thought that will always to lead to the keypress bug.  In reality the plP_eop() does something only half the time.


1) Plot starts
2) At BOP, the page_status is set to AT_BOP
3) While drawing, the page_status is set to DRAWING
4) At EOP, the page_status is set to AT_EOP  (the EOP is not put into the buffer)
5) The driver enters into a loop waiting for messages
6) Driver receives a resize message
7) plRemakePlot is called
8) plRemakPlot() calls plP_eop()
9) plP_eop() does nothing because it exits immediately because the page_status is AT_EOP
10) The plot buffer is replayed
11) At BOP, the page_status is set to AT_BOP
12) While drawing, the page_status is set to DRAWING
13) No EOP is generated because it is not in the buffer, thus the page_status is unchanged
14) Control returns the message loop
15) Another resize message is received
16) plRemakePlot is called
17) plRemakePlot() calls plP_eop()
18) plP_eop() does not exit immediately this time because page_state is DRAWING
19) plP_eop() calls the driver EOP handler
20) The driver enters into a new loop waiting for messages
21) User has to do an action (e.g. keypress) to exit the new message loop.  This blocks many messages because the windowing system still considers the window to be processing a resize.
22) Once the new message loop exits, the driver EOP handler exits.
23) Goto step 10.


Just putting the EOP into the plot buffer won’t fix the problem because a new message loop will be called, which will block messages.  


I agree with Phil’s point about the need to call plP_eop().  Some drivers may need the EOP handler to work correctly (e.g. double buffering). The way the code is structured, the plP_eop() is only doing something 50% of the time. 


I’m not sure the best way to fix this.  I think the interactive GUI drivers might need to be modified to check if they are already waiting.  One thing that makes this tricky is the strip chart function.  As it is currently written, it does not generate an EOP until the end.  Thus, it cannot be resized while plotting.  Plus, when resized, the entire strip chart is replayed (though I see Phil has made some commits so wxWidgets might be smarter).


I’m going to experiment on some different approaches and see what works best.

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 13, 2015, at 3:25 PM, Phil Rosenberg <p.d...@gm...> wrote:
> 
> Okay, well if you need any help or testing let me know.
> 
> Something that I haven't seen up to now (but maybe I never looked hard enough) is a spec sheet for how to write a driver, I.e. What events should a buffer deal with and in what way. Given the effort I've just been through with rewriting wxWidgets driver and what you are going through with the windows driver, perhaps we should write it, and then we can refer to it and if we wish to tidy the core-driver interface in the future it will be a useful reference.
> 

Absolutely. Writing a driver should not be this hard. I think getting things documented might clear some of the problems between what we think is happening and what is really happening. 

> Phil
> From: Jim Dishaw
> Sent: ‎13/‎06/‎2015 19:11
> To: Phil Rosenberg
> Cc: Alan W. Irwin; PLplot development list
> Subject: Re: [Plplot-devel] Bug fix to plbuf.c
> 
> 
>> On Jun 13, 2015, at 1:13 PM, Phil Rosenberg <p.d...@gm...> wrote:
>> 
>> Hmmm
>> Then hmmmm some more
>> Followed by an ummm or two.
>> 
>> As you say Jim, clearly things are more complicated than I realised. I'm not sure what the requirements really are here now. Now you have brought this up I can imagine the issues associated with entering the message loop after an eop.
>> 
>> In terms of dealing with it, I'm not at my computer right now, but I think probably a bop_forced function which forces a bop irrespective of where we are in the page cycle would work. But I'm not sure if that might have some potential subtle effects elsewhere so it is a bit of a hack really. Perhaps instead it would be better to assess what is being done in a bop and ensure those actions end up in the buffer so a bop event is not required.
> 
> I think that is a good approach for reasons beyond this issue, but a BOP and EOP of some type is still required to support the drivers adequately (e.g double buffering, printing from the windows API).  
> 
> The solution I'm going to test is one where the driver does not enter into a wait for input state on a redraw event from the callback. 
> 
>> Phil
>> From: Jim Dishaw
>> Sent: ‎13/‎06/‎2015 17:55
>> To: Alan W. Irwin
>> Cc: PLplot development list
>> Subject: Re: [Plplot-devel] Bug fix to plbuf.c
>> 
>> 
>>>> On Jun 13, 2015, at 12:03 PM, Jim Dishaw <ji...@di...> wrote:
>>>> 
>>>> 
>>>>> On Jun 11, 2015, at 12:29 AM, Jim Dishaw <ji...@di...> wrote:
>>>>> 
>>>>> 
>>>>> On Jun 10, 2015, at 11:30 PM, Alan W. Irwin <ir...@be...> wrote:
>>>  <snip>
>>>>> Just to confirm that I just now played a lot with resizing of
>>>>> 
>>>>> examples/c/x01c -dev xwin
>>>>> 
>>>>> for 5.10.0, 5.11.0, and git master tip (including your recent commit),
>>>>> and I found no specific string issues for any of those cases.  So it
>>>>> appears hard (at least with this example) to demonstrate a rendering
>>>>> bug due to this issue that you just fixed, but your fix seems logical
>>>>> to me (now) and certainly does not introduce any bad string rendering
>>>>> that I can spot.
>>>>> 
>>>>> However, when doing those checks I did notice other resizing issues
>>>>> that do need to be addressed.
>>>>> 
>>>>> 1. As a resize is occurring (typically by dragging one edge or one
>>>>> corner of the GUI to make the whole thing smaller or larger) sometimes
>>>>> the displayed GUI is a scaled plot and sometimes it is black (which is
>>>>> OK).  But on some occasions when I stop resizing (by releasing the
>>>>> mouse button) the result remains black for 5.11.0 and the master tip
>>>>> version (which is not OK).  But 5.10.0 is fine in this regard. So this
>>>>> issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
>>>>> and probably due to the flurry of changes you and Phil made in plbuf
>>>>> before we released 5.11.0.
>>>> 
>>>>> 
>>>>> Please verify the good result for 5.10.0 and bad result for 5.11.0,
>>>>> git bisect to figure out what commit has caused the issue, and then
>>>>> make the appropriate plbuf fix to get rid of this regression.
>>>> 
>>>> I think this problem is the plP_eop() that was inserted in the plRemakePlot() to make sure the BOP would perform as expected.  For the GUI drivers this will trigger another wait for input, which ends up blocking redraw messages.  I will verify by using git bisect.
>>> 
>>> I ran git bisect and the regression was introduced in the commit 1e402417c1f3e87c391fe428f936153c2a10e8cc
>>> 
>>> Author: Phil Rosenberg <p.d...@gm...>
>>> Date:   Fri Feb 27 17:12:03 2015 +0000
>>> 
>>>     Fixed bug in rdbuf_di.
>>>     
>>>     Save cursubpage on replot
>>>     call plP_eop() on replot which ensures that the first plP_bop() call actually does something.
>>> 
>>> If I comment out the plP_eop() that was added to plbuf.c in that commit, the xwin driver does not exhibit the problematic behavior.  I will investigate further on working on a patch.  I know why it is causing a problem, but it isn’t obvious to me how it gets to that point.
>> 
>> I found the cause of the problem and the plP_eop() is not acting in a way that neither Phil nor I expected.  Phil put the plP_eop() in to make sure that the BOP will do something and I thought that will always to lead to the keypress bug.  In reality the plP_eop() does something only half the time.
>> 
>> 1) Plot starts
>> 2) At BOP, the page_status is set to AT_BOP
>> 3) While drawing, the page_status is set to DRAWING
>> 4) At EOP, the page_status is set to AT_EOP  (the EOP is not put into the buffer)
>> 5) The driver enters into a loop waiting for messages
>> 6) Driver receives a resize message
>> 7) plRemakePlot is called
>> 8) plRemakPlot() calls plP_eop()
>> 9) plP_eop() does nothing because it exits immediately because the page_status is AT_EOP
>> 10) The plot buffer is replayed
>> 11) At BOP, the page_status is set to AT_BOP
>> 12) While drawing, the page_status is set to DRAWING
>> 13) No EOP is generated because it is not in the buffer, thus the page_status is unchanged
>> 14) Control returns the message loop
>> 15) Another resize message is received
>> 16) plRemakePlot is called
>> 17) plRemakePlot() calls plP_eop()
>> 18) plP_eop() does not exit immediately this time because page_state is DRAWING
>> 19) plP_eop() calls the driver EOP handler
>> 20) The driver enters into a new loop waiting for messages
>> 21) User has to do an action (e.g. keypress) to exit the new message loop.  This blocks many messages because the windowing system still considers the window to be processing a resize.
>> 22) Once the new message loop exits, the driver EOP handler exits.
>> 23) Goto step 10.
>> 
>> Just putting the EOP into the plot buffer won’t fix the problem because a new message loop will be called, which will block messages.  
>> 
>> I agree with Phil’s point about the need to call plP_eop().  Some drivers may need the EOP handler to work correctly (e.g. double buffering). The way the code is structured, the plP_eop() is only doing something 50% of the time. 
>> 
>> I’m not sure the best way to fix this.  I think the interactive GUI drivers might need to be modified to check if they are already waiting.  One thing that makes this tricky is the strip chart function.  As it is currently written, it does not generate an EOP until the end.  Thus, it cannot be resized while plotting.  Plus, when resized, the entire strip chart is replayed (though I see Phil has made some commits so wxWidgets might be smarter).
>> 
>> I’m going to experiment on some different approaches and see what works best.
>> 

Re: [Plplot-devel] Bug fix to plbuf.c

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

On 2015-06-13 17:15-0400 Jim Dishaw wrote:

>
>
>> On Jun 13, 2015, at 3:25 PM, Phil Rosenberg <p.d...@gm...> wrote:
>>
>> Okay, well if you need any help or testing let me know.
>>
>> Something that I haven't seen up to now (but maybe I never looked hard enough) is a spec sheet for how to write a driver, I.e. What events should a buffer deal with and in what way. Given the effort I've just been through with rewriting wxWidgets driver and what you are going through with the windows driver, perhaps we should write it, and then we can refer to it and if we wish to tidy the core-driver interface in the future it will be a useful reference.
>>
>
> Absolutely. Writing a driver should not be this hard. I think getting things documented might clear some of the problems between what we think is happening and what is really happening.

"Yes please" on some documentation for how to write a driver.  I am no
expert in this aspect of PLplot, but I do have two "overview" points to
make.

1. Device drivers for PLplot have been advertised as easy to write so let's make that so
with improved documentation.

2. My gut feeling is that the overview (aside from the X specifics)
for the xwin device would make a good template for an interactive
device driver.  The reason I say that is that was the interactive
device driver that got all the TLC in the early PLplot history. For
example, it handles calls to plxormod and plGetCursor properly for
example 20 while the other interactive device drivers currently do
not. So aside from treatment of text (which is way out of date for the
xwin device), I think it would be worth your while to study the design
overview of the xwin device to see if it might help you to document
the general design of interactive device drivers.

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
__________________________

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 13, 2015, at 2:11 PM, Jim Dishaw <ji...@di...> wrote:
> 
> 
> On Jun 13, 2015, at 1:13 PM, Phil Rosenberg <p.d...@gm... <mailto:p.d...@gm...>> wrote:
> 
>> Hmmm
>> Then hmmmm some more
>> Followed by an ummm or two.
>> 
>> As you say Jim, clearly things are more complicated than I realised. I'm not sure what the requirements really are here now. Now you have brought this up I can imagine the issues associated with entering the message loop after an eop.
>> 
>> In terms of dealing with it, I'm not at my computer right now, but I think probably a bop_forced function which forces a bop irrespective of where we are in the page cycle would work. But I'm not sure if that might have some potential subtle effects elsewhere so it is a bit of a hack really. Perhaps instead it would be better to assess what is being done in a bop and ensure those actions end up in the buffer so a bop event is not required.
>> 
> 
> I think that is a good approach for reasons beyond this issue, but a BOP and EOP of some type is still required to support the drivers adequately (e.g double buffering, printing from the windows API).  
> 
> The solution I'm going to test is one where the driver does not enter into a wait for input state on a redraw event from the callback. 

I came up with two solutions, one of which I tested.

Solution 1 (Tested)

Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so now an EOP is written to the buffer.  This will cause an EOP to be generated when the plot buffer is replayed.  I like the symmetry of having an EOP in the buffer to match the BOP.

Drivers will be responsible for determining if a “wait for user input” should be performed during an EOP.  For the wingdi driver I track the state of a “page_state” that changes outside of the BOP/EOP calls.

This solution feels a bit kludgy, but it works.

Solution 2

Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so now an EOP is written to the buffer.  This will cause an EOP to be generated when the plot buffer is replayed. 

Add a new driver function to the dispatch table (e.g. pl_wait) that calls the "wait for user input between pages" function.

Remove from all drivers the “wait for input” from the EOP handler.

The PLplot core library will call the wait for input between pages function (if not null).

This solution strikes me as a more elegant solution because the logic for deciding about the wait for input is one spot.

Regardless of the solution, some changes to the drivers will be required.  Solution 2 will touch all the drivers (at a minimum to add a NULL to the dispatch table).  Solution 1 adds code to the drivers while solution 2 removes some code from the drivers.

> 
>> Phil
>> From: Jim Dishaw <mailto:ji...@di...>
>> Sent: ‎13/‎06/‎2015 17:55
>> To: Alan W. Irwin <mailto:ir...@be...>
>> Cc: PLplot development list <mailto:Plp...@li...>
>> Subject: Re: [Plplot-devel] Bug fix to plbuf.c
>> 
>> 
>>> On Jun 13, 2015, at 12:03 PM, Jim Dishaw <ji...@di... <mailto:ji...@di...>> wrote:
>>> 
>>> 
>>>> On Jun 11, 2015, at 12:29 AM, Jim Dishaw <ji...@di... <mailto:ji...@di...>> wrote:
>>>> 
>>>>> 
>>>>> On Jun 10, 2015, at 11:30 PM, Alan W. Irwin <ir...@be... <mailto:ir...@be...>> wrote:
>>>  <snip>
>>>>> Just to confirm that I just now played a lot with resizing of
>>>>> 
>>>>> examples/c/x01c -dev xwin
>>>>> 
>>>>> for 5.10.0, 5.11.0, and git master tip (including your recent commit),
>>>>> and I found no specific string issues for any of those cases.  So it
>>>>> appears hard (at least with this example) to demonstrate a rendering
>>>>> bug due to this issue that you just fixed, but your fix seems logical
>>>>> to me (now) and certainly does not introduce any bad string rendering
>>>>> that I can spot.
>>>>> 
>>>>> However, when doing those checks I did notice other resizing issues
>>>>> that do need to be addressed.
>>>>> 
>>>>> 1. As a resize is occurring (typically by dragging one edge or one
>>>>> corner of the GUI to make the whole thing smaller or larger) sometimes
>>>>> the displayed GUI is a scaled plot and sometimes it is black (which is
>>>>> OK).  But on some occasions when I stop resizing (by releasing the
>>>>> mouse button) the result remains black for 5.11.0 and the master tip
>>>>> version (which is not OK).  But 5.10.0 is fine in this regard. So this
>>>>> issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
>>>>> and probably due to the flurry of changes you and Phil made in plbuf
>>>>> before we released 5.11.0.
>>>> 
>>>>> 
>>>>> Please verify the good result for 5.10.0 and bad result for 5.11.0,
>>>>> git bisect to figure out what commit has caused the issue, and then
>>>>> make the appropriate plbuf fix to get rid of this regression.
>>>>> 
>>>> 
>>>> I think this problem is the plP_eop() that was inserted in the plRemakePlot() to make sure the BOP would perform as expected.  For the GUI drivers this will trigger another wait for input, which ends up blocking redraw messages.  I will verify by using git bisect.
>>> 
>>> I ran git bisect and the regression was introduced in the commit 1e402417c1f3e87c391fe428f936153c2a10e8cc
>>> 
>>> Author: Phil Rosenberg <p.d...@gm... <mailto:p.d...@gm...>>
>>> Date:   Fri Feb 27 17:12:03 2015 +0000
>>> 
>>>     Fixed bug in rdbuf_di.
>>>     
>>>     Save cursubpage on replot
>>>     call plP_eop() on replot which ensures that the first plP_bop() call actually does something.
>>> 
>>> If I comment out the plP_eop() that was added to plbuf.c in that commit, the xwin driver does not exhibit the problematic behavior.  I will investigate further on working on a patch.  I know why it is causing a problem, but it isn’t obvious to me how it gets to that point.
>>> 
>> 
>> I found the cause of the problem and the plP_eop() is not acting in a way that neither Phil nor I expected.  Phil put the plP_eop() in to make sure that the BOP will do something and I thought that will always to lead to the keypress bug.  In reality the plP_eop() does something only half the time.
>> 
>> 1) Plot starts
>> 2) At BOP, the page_status is set to AT_BOP
>> 3) While drawing, the page_status is set to DRAWING
>> 4) At EOP, the page_status is set to AT_EOP  (the EOP is not put into the buffer)
>> 5) The driver enters into a loop waiting for messages
>> 6) Driver receives a resize message
>> 7) plRemakePlot is called
>> 8) plRemakPlot() calls plP_eop()
>> 9) plP_eop() does nothing because it exits immediately because the page_status is AT_EOP
>> 10) The plot buffer is replayed
>> 11) At BOP, the page_status is set to AT_BOP
>> 12) While drawing, the page_status is set to DRAWING
>> 13) No EOP is generated because it is not in the buffer, thus the page_status is unchanged
>> 14) Control returns the message loop
>> 15) Another resize message is received
>> 16) plRemakePlot is called
>> 17) plRemakePlot() calls plP_eop()
>> 18) plP_eop() does not exit immediately this time because page_state is DRAWING
>> 19) plP_eop() calls the driver EOP handler
>> 20) The driver enters into a new loop waiting for messages
>> 21) User has to do an action (e.g. keypress) to exit the new message loop.  This blocks many messages because the windowing system still considers the window to be processing a resize.
>> 22) Once the new message loop exits, the driver EOP handler exits.
>> 23) Goto step 10.
>> 
>> Just putting the EOP into the plot buffer won’t fix the problem because a new message loop will be called, which will block messages.  
>> 
>> I agree with Phil’s point about the need to call plP_eop().  Some drivers may need the EOP handler to work correctly (e.g. double buffering). The way the code is structured, the plP_eop() is only doing something 50% of the time. 
>> 
>> I’m not sure the best way to fix this.  I think the interactive GUI drivers might need to be modified to check if they are already waiting.  One thing that makes this tricky is the strip chart function.  As it is currently written, it does not generate an EOP until the end.  Thus, it cannot be resized while plotting.  Plus, when resized, the entire strip chart is replayed (though I see Phil has made some commits so wxWidgets might be smarter).
>> 
>> I’m going to experiment on some different approaches and see what works best.
>> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Plplot-devel mailing list
> Plp...@li... <mailto:Plp...@li...>
> https://lists.sourceforge.net/lists/listinfo/plplot-devel <https://lists.sourceforge.net/lists/listinfo/plplot-devel>

Re: [Plplot-devel] Bug fix to plbuf.c

[Phil R. <p.d...@gm...>]

I agree that option 2 seems less "hacky" - it is directly addressing
the problem with a clear solution rather than trying to bend the
existing interface and further muddy the interface between the core
and driver code.

The only disadvantage of 2 is that things will break (compile or run
time problems?) for driver that do not implement the new interface.

How many drivers are we talking about and would this effect
noninteractive drivers? The interactive drivers I can remember right
now are
wxWidgets
Qt
X
Windows
Tcl

What breakages will occur to them while we propagate this through?

Phil

On 14 June 2015 at 03:29, Jim Dishaw <ji...@di...> wrote:
>
> On Jun 13, 2015, at 2:11 PM, Jim Dishaw <ji...@di...> wrote:
>
>
> On Jun 13, 2015, at 1:13 PM, Phil Rosenberg <p.d...@gm...> wrote:
>
> Hmmm
> Then hmmmm some more
> Followed by an ummm or two.
>
> As you say Jim, clearly things are more complicated than I realised. I'm not
> sure what the requirements really are here now. Now you have brought this up
> I can imagine the issues associated with entering the message loop after an
> eop.
>
> In terms of dealing with it, I'm not at my computer right now, but I think
> probably a bop_forced function which forces a bop irrespective of where we
> are in the page cycle would work. But I'm not sure if that might have some
> potential subtle effects elsewhere so it is a bit of a hack really. Perhaps
> instead it would be better to assess what is being done in a bop and ensure
> those actions end up in the buffer so a bop event is not required.
>
>
> I think that is a good approach for reasons beyond this issue, but a BOP and
> EOP of some type is still required to support the drivers adequately (e.g
> double buffering, printing from the windows API).
>
> The solution I'm going to test is one where the driver does not enter into a
> wait for input state on a redraw event from the callback.
>
>
> I came up with two solutions, one of which I tested.
>
> Solution 1 (Tested)
>
> Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so
> now an EOP is written to the buffer.  This will cause an EOP to be generated
> when the plot buffer is replayed.  I like the symmetry of having an EOP in
> the buffer to match the BOP.
>
> Drivers will be responsible for determining if a “wait for user input”
> should be performed during an EOP.  For the wingdi driver I track the state
> of a “page_state” that changes outside of the BOP/EOP calls.
>
> This solution feels a bit kludgy, but it works.
>
> Solution 2
>
> Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so
> now an EOP is written to the buffer.  This will cause an EOP to be generated
> when the plot buffer is replayed.
>
> Add a new driver function to the dispatch table (e.g. pl_wait) that calls
> the "wait for user input between pages" function.
>
> Remove from all drivers the “wait for input” from the EOP handler.
>
> The PLplot core library will call the wait for input between pages function
> (if not null).
>
> This solution strikes me as a more elegant solution because the logic for
> deciding about the wait for input is one spot.
>
> Regardless of the solution, some changes to the drivers will be required.
> Solution 2 will touch all the drivers (at a minimum to add a NULL to the
> dispatch table).  Solution 1 adds code to the drivers while solution 2
> removes some code from the drivers.
>
>
> Phil
> ________________________________
> From: Jim Dishaw
> Sent: ‎13/‎06/‎2015 17:55
> To: Alan W. Irwin
> Cc: PLplot development list
> Subject: Re: [Plplot-devel] Bug fix to plbuf.c
>
>
> On Jun 13, 2015, at 12:03 PM, Jim Dishaw <ji...@di...> wrote:
>
>
> On Jun 11, 2015, at 12:29 AM, Jim Dishaw <ji...@di...> wrote:
>
>
> On Jun 10, 2015, at 11:30 PM, Alan W. Irwin <ir...@be...>
> wrote:
>
>  <snip>
>
> Just to confirm that I just now played a lot with resizing of
>
> examples/c/x01c -dev xwin
>
> for 5.10.0, 5.11.0, and git master tip (including your recent commit),
> and I found no specific string issues for any of those cases.  So it
> appears hard (at least with this example) to demonstrate a rendering
> bug due to this issue that you just fixed, but your fix seems logical
> to me (now) and certainly does not introduce any bad string rendering
> that I can spot.
>
> However, when doing those checks I did notice other resizing issues
> that do need to be addressed.
>
> 1. As a resize is occurring (typically by dragging one edge or one
> corner of the GUI to make the whole thing smaller or larger) sometimes
> the displayed GUI is a scaled plot and sometimes it is black (which is
> OK).  But on some occasions when I stop resizing (by releasing the
> mouse button) the result remains black for 5.11.0 and the master tip
> version (which is not OK).  But 5.10.0 is fine in this regard. So this
> issue is a regression introduced into PLplot between 5.10.0 and 5.11.0
> and probably due to the flurry of changes you and Phil made in plbuf
> before we released 5.11.0.
>
>
>
> Please verify the good result for 5.10.0 and bad result for 5.11.0,
> git bisect to figure out what commit has caused the issue, and then
> make the appropriate plbuf fix to get rid of this regression.
>
>
> I think this problem is the plP_eop() that was inserted in the
> plRemakePlot() to make sure the BOP would perform as expected.  For the GUI
> drivers this will trigger another wait for input, which ends up blocking
> redraw messages.  I will verify by using git bisect.
>
>
> I ran git bisect and the regression was introduced in the commit
> 1e402417c1f3e87c391fe428f936153c2a10e8cc
>
> Author: Phil Rosenberg <p.d...@gm...>
> Date:   Fri Feb 27 17:12:03 2015 +0000
>
>     Fixed bug in rdbuf_di.
>
>     Save cursubpage on replot
>     call plP_eop() on replot which ensures that the first plP_bop() call
> actually does something.
>
> If I comment out the plP_eop() that was added to plbuf.c in that commit, the
> xwin driver does not exhibit the problematic behavior.  I will investigate
> further on working on a patch.  I know why it is causing a problem, but it
> isn’t obvious to me how it gets to that point.
>
>
> I found the cause of the problem and the plP_eop() is not acting in a way
> that neither Phil nor I expected.  Phil put the plP_eop() in to make sure
> that the BOP will do something and I thought that will always to lead to the
> keypress bug.  In reality the plP_eop() does something only half the time.
>
> 1) Plot starts
> 2) At BOP, the page_status is set to AT_BOP
> 3) While drawing, the page_status is set to DRAWING
> 4) At EOP, the page_status is set to AT_EOP  (the EOP is not put into the
> buffer)
> 5) The driver enters into a loop waiting for messages
> 6) Driver receives a resize message
> 7) plRemakePlot is called
> 8) plRemakPlot() calls plP_eop()
> 9) plP_eop() does nothing because it exits immediately because the
> page_status is AT_EOP
> 10) The plot buffer is replayed
> 11) At BOP, the page_status is set to AT_BOP
> 12) While drawing, the page_status is set to DRAWING
> 13) No EOP is generated because it is not in the buffer, thus the
> page_status is unchanged
> 14) Control returns the message loop
> 15) Another resize message is received
> 16) plRemakePlot is called
> 17) plRemakePlot() calls plP_eop()
> 18) plP_eop() does not exit immediately this time because page_state is
> DRAWING
> 19) plP_eop() calls the driver EOP handler
> 20) The driver enters into a new loop waiting for messages
> 21) User has to do an action (e.g. keypress) to exit the new message loop.
> This blocks many messages because the windowing system still considers the
> window to be processing a resize.
> 22) Once the new message loop exits, the driver EOP handler exits.
> 23) Goto step 10.
>
> Just putting the EOP into the plot buffer won’t fix the problem because a
> new message loop will be called, which will block messages.
>
> I agree with Phil’s point about the need to call plP_eop().  Some drivers
> may need the EOP handler to work correctly (e.g. double buffering). The way
> the code is structured, the plP_eop() is only doing something 50% of the
> time.
>
> I’m not sure the best way to fix this.  I think the interactive GUI drivers
> might need to be modified to check if they are already waiting.  One thing
> that makes this tricky is the strip chart function.  As it is currently
> written, it does not generate an EOP until the end.  Thus, it cannot be
> resized while plotting.  Plus, when resized, the entire strip chart is
> replayed (though I see Phil has made some commits so wxWidgets might be
> smarter).
>
> I’m going to experiment on some different approaches and see what works
> best.
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Plplot-devel mailing list
> Plp...@li...
> https://lists.sourceforge.net/lists/listinfo/plplot-devel
>
>

Re: [Plplot-devel] Bug fix to plbuf.c

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

On 2015-06-13 22:29-0400 Jim Dishaw wrote:

>
>> On Jun 13, 2015, at 2:11 PM, Jim Dishaw <ji...@di...> wrote:
>>
>>
>> On Jun 13, 2015, at 1:13 PM, Phil Rosenberg <p.d...@gm... <mailto:p.d...@gm...>> wrote:
>>
>>> Hmmm
>>> Then hmmmm some more
>>> Followed by an ummm or two.
>>>
>>> As you say Jim, clearly things are more complicated than I realised. I'm not sure what the requirements really are here now. Now you have brought this up I can imagine the issues associated with entering the message loop after an eop.
>>>
>>> In terms of dealing with it, I'm not at my computer right now, but I think probably a bop_forced function which forces a bop irrespective of where we are in the page cycle would work. But I'm not sure if that might have some potential subtle effects elsewhere so it is a bit of a hack really. Perhaps instead it would be better to assess what is being done in a bop and ensure those actions end up in the buffer so a bop event is not required.
>>>
>>
>> I think that is a good approach for reasons beyond this issue, but a BOP and EOP of some type is still required to support the drivers adequately (e.g double buffering, printing from the windows API).
>>
>> The solution I'm going to test is one where the driver does not enter into a wait for input state on a redraw event from the callback.
>
> I came up with two solutions, one of which I tested.
>
> Solution 1 (Tested)
>
> Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so now an EOP is written to the buffer.  This will cause an EOP to be generated when the plot buffer is replayed.  I like the symmetry of having an EOP in the buffer to match the BOP.
>
> Drivers will be responsible for determining if a “wait for user input” should be performed during an EOP.  For the wingdi driver I track the state of a “page_state” that changes outside of the BOP/EOP calls.
>
> This solution feels a bit kludgy, but it works.
>
> Solution 2
>
> Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so now an EOP is written to the buffer.  This will cause an EOP to be generated when the plot buffer is replayed.
>
> Add a new driver function to the dispatch table (e.g. pl_wait) that calls the "wait for user input between pages" function.
>
> Remove from all drivers the “wait for input” from the EOP handler.
>
> The PLplot core library will call the wait for input between pages function (if not null).
>
> This solution strikes me as a more elegant solution because the logic for deciding about the wait for input is one spot.
>

> Regardless of the solution, some changes to the drivers will be
required.  Solution 2 will touch all the drivers (at a minimum to add
a NULL to the dispatch table).  Solution 1 adds code to the drivers
while solution 2 removes some code from the drivers.

Hi Jim:

Thanks for your effort working through possible solutions to the
problem.

@Phil and Andrew:

Assuming you guys agree with Jim's analysis, do you prefer solution 1
or 2?  Elegant and "removes some code from the drivers" (i.e.,
solution 2) seems like the better choice to me from an overview
perspective, but I would be happy to go along with whatever you all
decide since I do not have the driver expertise you guys have.

Assuming whatever solution is decided upon here can be implemented
in a reasonably timely way, then I think we should be able to fit
it into the forthcoming bug-fix release since this is obviously a
bug fix, albeit a rather intrusive one.

With regard to timing for that release there is still quite a bit I
plan to do to work through the bugs in the build system revealed by
previous tests on Linux and Arjen's current tests on the Cygwin
platform. And the new tarball report that is automatically collected
by scripts/comprehensive_test.sh should make reporting of tests and
figuring out build-system issues from those reports _much_ easier.
Which likely means this release will be tested on a lot more platforms
than the last one and a significant amount of time will be required to
deal with any build system bugs that are turned up by such tests. So
my current guestimate is the forthcoming release is likely more than a
month away (although hopefully not longer than two months away).

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
__________________________

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 14, 2015, at 5:19 AM, Alan W. Irwin <ir...@be...> wrote:
> 
> On 2015-06-13 22:29-0400 Jim Dishaw wrote:
> 
>> 
>>> On Jun 13, 2015, at 2:11 PM, Jim Dishaw <ji...@di...> wrote:
>>> 
>>> 
>>> On Jun 13, 2015, at 1:13 PM, Phil Rosenberg <p.d...@gm... <mailto:p.d...@gm...>> wrote:
>>> 
>>>> Hmmm
>>>> Then hmmmm some more
>>>> Followed by an ummm or two.
>>>> 
>>>> As you say Jim, clearly things are more complicated than I realised. I'm not sure what the requirements really are here now. Now you have brought this up I can imagine the issues associated with entering the message loop after an eop.
>>>> 
>>>> In terms of dealing with it, I'm not at my computer right now, but I think probably a bop_forced function which forces a bop irrespective of where we are in the page cycle would work. But I'm not sure if that might have some potential subtle effects elsewhere so it is a bit of a hack really. Perhaps instead it would be better to assess what is being done in a bop and ensure those actions end up in the buffer so a bop event is not required.
>>>> 
>>> 
>>> I think that is a good approach for reasons beyond this issue, but a BOP and EOP of some type is still required to support the drivers adequately (e.g double buffering, printing from the windows API).
>>> 
>>> The solution I'm going to test is one where the driver does not enter into a wait for input state on a redraw event from the callback.
>> 
>> I came up with two solutions, one of which I tested.
>> 
>> Solution 1 (Tested)
>> 
>> Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so now an EOP is written to the buffer.  This will cause an EOP to be generated when the plot buffer is replayed.  I like the symmetry of having an EOP in the buffer to match the BOP.
>> 
>> Drivers will be responsible for determining if a “wait for user input” should be performed during an EOP.  For the wingdi driver I track the state of a “page_state” that changes outside of the BOP/EOP calls.
>> 
>> This solution feels a bit kludgy, but it works.
>> 
>> Solution 2
>> 
>> Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so now an EOP is written to the buffer.  This will cause an EOP to be generated when the plot buffer is replayed.
>> 
>> Add a new driver function to the dispatch table (e.g. pl_wait) that calls the "wait for user input between pages" function.
>> 
>> Remove from all drivers the “wait for input” from the EOP handler.
>> 
>> The PLplot core library will call the wait for input between pages function (if not null).
>> 
>> This solution strikes me as a more elegant solution because the logic for deciding about the wait for input is one spot.
>> 
> 
>> Regardless of the solution, some changes to the drivers will be
> required.  Solution 2 will touch all the drivers (at a minimum to add
> a NULL to the dispatch table).  Solution 1 adds code to the drivers
> while solution 2 removes some code from the drivers.
> 
> Hi Jim:
> 
> Thanks for your effort working through possible solutions to the
> problem.
> 
> @Phil and Andrew:
> 
> Assuming you guys agree with Jim's analysis, do you prefer solution 1
> or 2?  Elegant and "removes some code from the drivers" (i.e.,
> solution 2) seems like the better choice to me from an overview
> perspective, but I would be happy to go along with whatever you all
> decide since I do not have the driver expertise you guys have.
> 
> Assuming whatever solution is decided upon here can be implemented
> in a reasonably timely way, then I think we should be able to fit
> it into the forthcoming bug-fix release since this is obviously a
> bug fix, albeit a rather intrusive one.
> 
> With regard to timing for that release there is still quite a bit I
> plan to do to work through the bugs in the build system revealed by
> previous tests on Linux and Arjen's current tests on the Cygwin
> platform. And the new tarball report that is automatically collected
> by scripts/comprehensive_test.sh should make reporting of tests and
> figuring out build-system issues from those reports _much_ easier.
> Which likely means this release will be tested on a lot more platforms
> than the last one and a significant amount of time will be required to
> deal with any build system bugs that are turned up by such tests. So
> my current guestimate is the forthcoming release is likely more than a
> month away (although hopefully not longer than two months away).
> 

The drivers that look at the npause of the PLStream structure are:

drivers/cairo.c
drivers/deprecated_wxwidgets.cpp
drivers/linuxvga.c
drivers/mem.c
drivers/ntk.c
drivers/pbm.c
drivers/qt.cpp
drivers/tek.c
drivers/tk.c
drivers/tkwin.c
drivers/wingcc.c
drivers/xwin.c

Of those, the ones that use plRemakePlot() or the plot buffer are:

drivers/tkwin.c
drivers/wingcc.c
drivers/xwin.c
The new wxWidgets drivers

Thus, implementing the second solution is fairly trivial for the non-plot buffer drivers (just need to split the EOP handler)

Looking at the relevant files:

The easy to split: cairo, linuxvga, qt, tk, twin, wingcc, xwin

The “needs some thought”:
  ntk - driver pauses when the stream is destroyed (in the plD_tidy_ntk function rather than the EOP handler).
  tek - will need to move the CLEAR_VIEW in addition to splitting

The false positives (they don’t actually pause, they set nopause to true): men, pbm

The aqt driver is a mystery.  It does not use the plot buffer nor does it look at the nopause member; however, it is an interactive driver. It should work with just a NULL in the wait handler because the EOP will be called only once.  The dg300 is another example.

 

Re: [Plplot-devel] Bug fix to plbuf.c

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

On 2015-06-14 13:40-0400 Jim Dishaw wrote:

> [....]The aqt driver is a mystery.  It does not use the plot buffer
nor does it look at the nopause member; however, it is an interactive
driver. It should work with just a NULL in the wait handler because
the EOP will be called only once.  The dg300 is another example.

Hi Jim:

See cmake/modules/drivers-init.cmake for the status of each device
driver.  Some of them (e.g., dg300) have been completely retired
because they are unmaintained and likely not compatible with modern
PLplot any more.  We should completely remove the source code for
every retired PLplot device such as dg300 so as not to have old
incompatible driver code obfuscating the source code for devices that
are being actively maintained.

@Hazen:
Have you given up on maintaining aqt, i.e., is it time to retire that
driver and completely remove its source code?

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
__________________________

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 14, 2015, at 5:59 PM, Alan W. Irwin <ir...@be...> wrote:
> 
>> On 2015-06-14 13:40-0400 Jim Dishaw wrote:
>> 
>> [....]The aqt driver is a mystery.  It does not use the plot buffer
> nor does it look at the nopause member; however, it is an interactive
> driver. It should work with just a NULL in the wait handler because
> the EOP will be called only once.  The dg300 is another example.
> 
> Hi Jim:
> 
> See cmake/modules/drivers-init.cmake for the status of each device
> driver.  Some of them (e.g., dg300) have been completely retired
> because they are unmaintained and likely not compatible with modern
> PLplot any more.  We should completely remove the source code for
> every retired PLplot device such as dg300 so as not to have old
> incompatible driver code obfuscating the source code for devices that
> are being actively maintained.
> 
> @Hazen:
> Have you given up on maintaining aqt, i.e., is it time to retire that
> driver and completely remove its source code?
> 

I am planning on making a native driver for macos x, though I am not sure if I can make the cutoff for 5.11.1

Re: [Plplot-devel] Bug fix to plbuf.c

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

On 2015-06-14 19:16-0400 Jim Dishaw wrote:

> I am planning on making a native driver for macos x, though I am not
sure if I can make the cutoff for 5.11.1

Hi Jim:

Such a native Mac OS X device would be quite welcome.  However, that's
new development where the cutoff for pushing to the master branch is
roughly one month after the latest release.  The reason why I decided
to assert that policy as release manager is to provide plenty of
testing time during a release cycle for any new development.

So we are long past that cutoff for this release cycle, but I still
strongly encourage merging bug fixes (like one of your two suggestions
for fixing the eop bug) and documentation (e.g., the documentation for
developers of modern devices that was discussed) to master for quite a
while longer during this current release cycle.

With regard to new development such as the new GDI and Mac OS X
devices, ideally you should develop and mature them now on private
development branches so you will be prepared to merge them to master
just after the release of 5.11.1 (and certainly within the first month
after that date).

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
__________________________

Re: [Plplot-devel] Bug fix to plbuf.c

[Hazen B. <hba...@ma...>]

On 06/14/2015 05:59 PM, Alan W. Irwin wrote:
> On 2015-06-14 13:40-0400 Jim Dishaw wrote:
>
>> [....]The aqt driver is a mystery.  It does not use the plot buffer
> nor does it look at the nopause member; however, it is an interactive
> driver. It should work with just a NULL in the wait handler because
> the EOP will be called only once.  The dg300 is another example.
>
> Hi Jim:
>
> See cmake/modules/drivers-init.cmake for the status of each device
> driver.  Some of them (e.g., dg300) have been completely retired
> because they are unmaintained and likely not compatible with modern
> PLplot any more.  We should completely remove the source code for
> every retired PLplot device such as dg300 so as not to have old
> incompatible driver code obfuscating the source code for devices that
> are being actively maintained.
>
> @Hazen:
> Have you given up on maintaining aqt, i.e., is it time to retire that
> driver and completely remove its source code?

I am not maintaining it, but I believe that it still works so I would 
not remove it.

-Hazen

Re: [Plplot-devel] Bug fix to plbuf.c

[Jim D. <ji...@di...>]

> On Jun 14, 2015, at 1:40 PM, Jim Dishaw <ji...@di...> wrote:
> 
> 
>> On Jun 14, 2015, at 5:19 AM, Alan W. Irwin <ir...@be...> wrote:
>> 
>> On 2015-06-13 22:29-0400 Jim Dishaw wrote:
>> 
>>> 
>>>> On Jun 13, 2015, at 2:11 PM, Jim Dishaw <ji...@di...> wrote:
>>>> 
>>>> 
>>>> On Jun 13, 2015, at 1:13 PM, Phil Rosenberg <p.d...@gm... <mailto:p.d...@gm...>> wrote:
>>>> 
>>>>> Hmmm
>>>>> Then hmmmm some more
>>>>> Followed by an ummm or two.
>>>>> 
>>>>> As you say Jim, clearly things are more complicated than I realised. I'm not sure what the requirements really are here now. Now you have brought this up I can imagine the issues associated with entering the message loop after an eop.
>>>>> 
>>>>> In terms of dealing with it, I'm not at my computer right now, but I think probably a bop_forced function which forces a bop irrespective of where we are in the page cycle would work. But I'm not sure if that might have some potential subtle effects elsewhere so it is a bit of a hack really. Perhaps instead it would be better to assess what is being done in a bop and ensure those actions end up in the buffer so a bop event is not required.
>>>>> 
>>>> 
>>>> I think that is a good approach for reasons beyond this issue, but a BOP and EOP of some type is still required to support the drivers adequately (e.g double buffering, printing from the windows API).
>>>> 
>>>> The solution I'm going to test is one where the driver does not enter into a wait for input state on a redraw event from the callback.
>>> 
>>> I came up with two solutions, one of which I tested.
>>> 
>>> Solution 1 (Tested)
>>> 
>>> Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so now an EOP is written to the buffer.  This will cause an EOP to be generated when the plot buffer is replayed.  I like the symmetry of having an EOP in the buffer to match the BOP.
>>> 
>>> Drivers will be responsible for determining if a “wait for user input” should be performed during an EOP.  For the wingdi driver I track the state of a “page_state” that changes outside of the BOP/EOP calls.
>>> 
>>> This solution feels a bit kludgy, but it works.
>>> 
>>> Solution 2
>>> 
>>> Remove the plP_eop() call in plRemakePlot().  Add an EOP to plbuf_eop() so now an EOP is written to the buffer.  This will cause an EOP to be generated when the plot buffer is replayed.
>>> 
>>> Add a new driver function to the dispatch table (e.g. pl_wait) that calls the "wait for user input between pages" function.
>>> 
>>> Remove from all drivers the “wait for input” from the EOP handler.
>>> 
>>> The PLplot core library will call the wait for input between pages function (if not null).
>>> 
>>> This solution strikes me as a more elegant solution because the logic for deciding about the wait for input is one spot.
>>> 
>> 
>>> Regardless of the solution, some changes to the drivers will be
>> required.  Solution 2 will touch all the drivers (at a minimum to add
>> a NULL to the dispatch table).  Solution 1 adds code to the drivers
>> while solution 2 removes some code from the drivers.
>> 
>> Hi Jim:
>> 
>> Thanks for your effort working through possible solutions to the
>> problem.
>> 
>> @Phil and Andrew:
>> 
>> Assuming you guys agree with Jim's analysis, do you prefer solution 1
>> or 2?  Elegant and "removes some code from the drivers" (i.e.,
>> solution 2) seems like the better choice to me from an overview
>> perspective, but I would be happy to go along with whatever you all
>> decide since I do not have the driver expertise you guys have.
>> 
>> Assuming whatever solution is decided upon here can be implemented
>> in a reasonably timely way, then I think we should be able to fit
>> it into the forthcoming bug-fix release since this is obviously a
>> bug fix, albeit a rather intrusive one.
>> 
>> With regard to timing for that release there is still quite a bit I
>> plan to do to work through the bugs in the build system revealed by
>> previous tests on Linux and Arjen's current tests on the Cygwin
>> platform. And the new tarball report that is automatically collected
>> by scripts/comprehensive_test.sh should make reporting of tests and
>> figuring out build-system issues from those reports _much_ easier.
>> Which likely means this release will be tested on a lot more platforms
>> than the last one and a significant amount of time will be required to
>> deal with any build system bugs that are turned up by such tests. So
>> my current guestimate is the forthcoming release is likely more than a
>> month away (although hopefully not longer than two months away).
>> 

Attached is a patch series that implements the second solution.  I tested the changes to xwin driver; however, I was not able to test tkwin, qt, or cairo.  I will provide an update to wingcc separately.  I didn’t touch the wxwidgets driver.

I didn’t have to touch the drivers that do not need the wait function pointer (e.g. postscript).  The way the driver dispatch table is initialized, I was able to set all the function pointers to NULL in the plcore.c file.  The drivers set each member individually, thus the wait function pointer is left NULL.

The xwin driver occasionally misses some resizes, but I think that is some quirky behavior that existed prior to 5.10.  

If someone could test the tkwin, qt, and cairo (specifically xcairo) that would be greatly appreciated.

Re: [Plplot-devel] Bug fix to plbuf.c

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

On 2015-06-15 12:24-0400 Jim Dishaw wrote:

> [...] Attached is a patch series that implements the second
[elegant] solution.  I tested the changes to xwin driver; however, I
was not able to test tkwin, qt, or cairo.  I will provide an update to
wingcc separately.  I didn’t touch the wxwidgets driver.

> I didn’t have to touch the drivers that do not need the wait
function pointer (e.g. postscript).  The way the driver dispatch table
is initialized, I was able to set all the function pointers to NULL in
the plcore.c file.  The drivers set each member individually, thus the
wait function pointer is left NULL.

> The xwin driver occasionally misses some resizes, but I think that is some quirky behavior that existed prior to 5.10.

> If someone could test the tkwin, qt, and cairo (specifically xcairo) that would be greatly appreciated.

Hi Jim:

Thanks for this effort and the later effort you posted for the wingcc
device driver.

@Jim and Phil:

I am in the middle of another PLplot issue at the moment, but I will test
Jim's fix for all of tkwin, qt, and cairo once I am done with that
issue.

I noticed above that Jim have no plans for the wxwidgets driver.  But
I assume we will need that device fixed and also the old wxwidgets
device you get when using -DOLD_WXWIDGETS=ON before we can push these
changes. So I hope if the bug fix is simple for both forms of wxwidgets
that Jim just goes ahead with it, but if it is more complex, I hope
both you guys are collaborating on making the fix.

Anyhow, once I see a patch series for the two forms of wxwidgets from
one or the other of you, I would be happy to test both those forms.

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
__________________________