Menu
▾
▴
gnuplot-beta
|
From: Petr M. <mi...@ph...> - 2009-03-16 22:37:43
|
In Windows, there is only one open and active graph window.
I think this could be the reason while mouse.c:event_keypress() reports
"protocol error" if you try e.g.:
bind 's' 'print "Hello";'
plot x*x
and then press "s" hotkey several times.
The code after
if (ptr->allwindows && ptr->command) {
is testing "allwindows" and "active" windows but it could somehow miss that
case of a single-window graph. I'm not sure what should be the correct fix
among these several if .. else if ...
---
PM
|
|
From: Petr M. <mi...@ph...> - 2009-03-19 05:55:05
|
Can somebody else on Windows reproduce the bug below? I would like this gets
fixed before 4.2.5 is released.
In Windows, there is only one open and active graph window.
I think this could be the reason while mouse.c:event_keypress() reports
"protocol error" if you try e.g.:
bind 's' 'print "Hello";'
bind 'd' 'set grid;'
bind 'f' 'set time'
bind 'g' 'plot x;'
bind 'x' 'test;'
plot x*x
and then press "s" hotkey (or the others) several times.
The code in mouse.c after
if (ptr->allwindows && ptr->command) {
is testing "allwindows" and "active" windows but it could somehow miss the
case of a single-window graph. I'm not sure what should be the correct fix
among these several if .. else if ... This well handles case of multiple
windows (x11, wxt), but what should it do for single-window cases such as
windows or pm terminals?
---
PM
|
|
From: Ethan A M. <merritt@u.washington.edu> - 2009-03-19 06:40:37
|
On Wednesday 18 March 2009, Petr Mikulik wrote:
> Can somebody else on Windows reproduce the bug below? I would like this gets
> fixed before 4.2.5 is released.
>
> In Windows, there is only one open and active graph window.
> I think this could be the reason while mouse.c:event_keypress() reports
> "protocol error" if you try e.g.:
>
> bind 's' 'print "Hello";'
> bind 'd' 'set grid;'
> bind 'f' 'set time'
> bind 'g' 'plot x;'
> bind 'x' 'test;'
> plot x*x
>
> and then press "s" hotkey (or the others) several times.
>
> The code in mouse.c after
> if (ptr->allwindows && ptr->command) {
>
> is testing "allwindows" and "active" windows but it could somehow miss the
> case of a single-window graph. I'm not sure what should be the correct fix
> among these several if .. else if ... This well handles case of multiple
> windows (x11, wxt), but what should it do for single-window cases such as
> windows or pm terminals?
It must recognize the current window correctly, or it would have exited the
loop before reaching that protocol error message.
1387: } else if (!current) break;
I think it is more likely that some spurious event is being generated
in addition to the desired keypress event, and we should just ignore it.
Is the behaviour acceptable if you change the fprintf() to FPRINTF(())?
--
Ethan A Merritt
|
|
From: Petr M. <mi...@ph...> - 2009-03-19 22:32:49
|
> > Can somebody else on Windows reproduce the bug below? I would like this gets
> > fixed before 4.2.5 is released.
> >
> > In Windows, there is only one open and active graph window.
> > I think this could be the reason while mouse.c:event_keypress() reports
> > "protocol error" if you try e.g.:
> >
> > bind 's' 'print "Hello";'
> > bind 'd' 'set grid;'
> > bind 'f' 'set time'
> > bind 'g' 'plot x;'
> > bind 'x' 'test;'
> > plot x*x
> >
> > and then press "s" hotkey (or the others) several times.
> >
> > The code in mouse.c after
> > if (ptr->allwindows && ptr->command) {
> >
> > is testing "allwindows" and "active" windows but it could somehow miss the
> > case of a single-window graph. I'm not sure what should be the correct fix
> > among these several if .. else if ... This well handles case of multiple
> > windows (x11, wxt), but what should it do for single-window cases such as
> > windows or pm terminals?
>
> It must recognize the current window correctly, or it would have exited the
> loop before reaching that protocol error message.
> 1387: } else if (!current) break;
>
> I think it is more likely that some spurious event is being generated
> in addition to the desired keypress event, and we should just ignore it.
> Is the behaviour acceptable if you change the fprintf() to FPRINTF(())?
Correct behaviour is achieved by setting
ptr->allwindows = 1
or by
par2 = 0
With
current = 1
some events are lost and "protocol error" is shown.
What (and where) should be set for correct behaviour?
---
PM
|
|
From: Ethan M. <merritt@u.washington.edu> - 2009-03-20 00:01:52
|
On Thursday 19 March 2009 15:32:41 Petr Mikulik wrote:
> > > Can somebody else on Windows reproduce the bug below? I would like this gets
> > > fixed before 4.2.5 is released.
> > >
> > > In Windows, there is only one open and active graph window.
> > > I think this could be the reason while mouse.c:event_keypress() reports
> > > "protocol error" if you try e.g.:
> > >
> > > bind 's' 'print "Hello";'
> > > bind 'd' 'set grid;'
> > > bind 'f' 'set time'
> > > bind 'g' 'plot x;'
> > > bind 'x' 'test;'
> > > plot x*x
> > >
> > > and then press "s" hotkey (or the others) several times.
> > >
> > > The code in mouse.c after
> > > if (ptr->allwindows && ptr->command) {
> > >
> > > is testing "allwindows" and "active" windows but it could somehow miss the
> > > case of a single-window graph. I'm not sure what should be the correct fix
> > > among these several if .. else if ... This well handles case of multiple
> > > windows (x11, wxt), but what should it do for single-window cases such as
> > > windows or pm terminals?
> >
> > It must recognize the current window correctly, or it would have exited the
> > loop before reaching that protocol error message.
> > 1387: } else if (!current) break;
> >
> > I think it is more likely that some spurious event is being generated
> > in addition to the desired keypress event, and we should just ignore it.
> > Is the behaviour acceptable if you change the fprintf() to FPRINTF(())?
>
> Correct behaviour is achieved by setting
> ptr->allwindows = 1
> or by
> par2 = 0
> With
> current = 1
> some events are lost and "protocol error" is shown.
>
> What (and where) should be set for correct behaviour?
OK, with that set of clues I conclude that the failure happens whenever
the GE_keypress event notification is sent with a non-zero par2.
The only place that can happen is here, and only when
thisTimestamp != lastTimestamp. Suggested patch is shown below.
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
--- gnuplot42/src/win/wgraph.c 2008-10-10 10:21:07.000000000 -0700
+++ gnuplot-4.2.5/src/win/wgraph.c 2009-03-19 16:59:30.000000000 -0700
@@ -1973,9 +1973,13 @@ Wnd_exec_event(LPGW lpgw, LPARAM lparam,
int mx, my;
static unsigned long lastTimestamp = 0;
unsigned long thisTimestamp = GetMessageTime();
+ int par2 = thisTimestamp - lastTimestamp;
+
+ if (type == GE_keypress)
+ par2 = 0;
GetMousePosViewport(lpgw, &mx, &my);
- gp_exec_event(type, mx, my, par1, thisTimestamp - lastTimestamp, 0);
+ gp_exec_event(type, mx, my, par1, par2, 0);
lastTimestamp = thisTimestamp;
}
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
I do not know what this Timestamp test is intended to do.
No equivalent test exists in the x11 or wxt drivers for keypress events.
Perhaps it is intended to handle double-click? If so it should only
apply to buttonrelease events, not keypress events.
It might also be safe to simply remove the test in mouse.c, like this:
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
--- gnuplot42/src/mouse.c 2009-02-08 20:43:44.000000000 -0800
+++ gnuplot-4.2.5/src/mouse.c 2009-03-19 16:51:07.000000000 -0700
@@ -1387,7 +1387,7 @@ event_keypress(struct gp_event_t *ge, TB
} else if (!current) {
break;
/* Let user defined bindings overwrite the builtin bindings */
- } else if ((par2 & 1) == 0 && ptr->command) {
+ } else if (ptr->command) {
do_string(ptr->command);
break;
} else if (ptr->builtin) {
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
but I am a little reluctant to do that for 4.2.5 because I haven't figured
out what case it was supposed to handle. We could make the change in 4.3
and see if anyone reports problems with x11 or wxt or os2.
--
Ethan A Merritt
|
|
From: Petr M. <mi...@ph...> - 2009-03-23 22:17:47
|
> > > > Can somebody else on Windows reproduce the bug below? I would like this gets
> > > > fixed before 4.2.5 is released.
> > > >
> > > > In Windows, there is only one open and active graph window.
> > > > I think this could be the reason while mouse.c:event_keypress() reports
> > > > "protocol error" if you try e.g.:
> > > >
> > > > bind 's' 'print "Hello";'
> > > > bind 'd' 'set grid;'
> > > > bind 'f' 'set time'
> > > > bind 'g' 'plot x;'
> > > > bind 'x' 'test;'
> > > > plot x*x
> > > >
> > > > and then press "s" hotkey (or the others) several times.
> > > >
> > > > The code in mouse.c after
> > > > if (ptr->allwindows && ptr->command) {
> > > >
> > > > is testing "allwindows" and "active" windows but it could somehow miss the
> > > > case of a single-window graph. I'm not sure what should be the correct fix
> > > > among these several if .. else if ... This well handles case of multiple
> > > > windows (x11, wxt), but what should it do for single-window cases such as
> > > > windows or pm terminals?
> > >
> > > It must recognize the current window correctly, or it would have exited the
> > > loop before reaching that protocol error message.
> > > 1387: } else if (!current) break;
> > >
> > > I think it is more likely that some spurious event is being generated
> > > in addition to the desired keypress event, and we should just ignore it.
> > > Is the behaviour acceptable if you change the fprintf() to FPRINTF(())?
> >
> > Correct behaviour is achieved by setting
> > ptr->allwindows = 1
> > or by
> > par2 = 0
> > With
> > current = 1
> > some events are lost and "protocol error" is shown.
> >
> > What (and where) should be set for correct behaviour?
>
> OK, with that set of clues I conclude that the failure happens whenever
> the GE_keypress event notification is sent with a non-zero par2.
> The only place that can happen is here, and only when
> thisTimestamp != lastTimestamp. Suggested patch is shown below.
>
> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
>
> --- gnuplot42/src/win/wgraph.c 2008-10-10 10:21:07.000000000 -0700
> +++ gnuplot-4.2.5/src/win/wgraph.c 2009-03-19 16:59:30.000000000 -0700
> @@ -1973,9 +1973,13 @@ Wnd_exec_event(LPGW lpgw, LPARAM lparam,
> int mx, my;
> static unsigned long lastTimestamp = 0;
> unsigned long thisTimestamp = GetMessageTime();
> + int par2 = thisTimestamp - lastTimestamp;
> +
> + if (type == GE_keypress)
> + par2 = 0;
>
> GetMousePosViewport(lpgw, &mx, &my);
> - gp_exec_event(type, mx, my, par1, thisTimestamp - lastTimestamp, 0);
> + gp_exec_event(type, mx, my, par1, par2, 0);
> lastTimestamp = thisTimestamp;
> }
This patch works OK, thanks!
> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
>
> I do not know what this Timestamp test is intended to do.
> No equivalent test exists in the x11 or wxt drivers for keypress events.
> Perhaps it is intended to handle double-click? If so it should only
> apply to buttonrelease events, not keypress events.
There are doubleclicks, see "help mouse".
MB1 copies mouse position to clipboard. This needs to be a doubleclick
because single click focuses/selects the window.
> It might also be safe to simply remove the test in mouse.c, like this:
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> --- gnuplot42/src/mouse.c 2009-02-08 20:43:44.000000000 -0800
> +++ gnuplot-4.2.5/src/mouse.c 2009-03-19 16:51:07.000000000 -0700
> @@ -1387,7 +1387,7 @@ event_keypress(struct gp_event_t *ge, TB
> } else if (!current) {
> break;
> /* Let user defined bindings overwrite the builtin bindings */
> - } else if ((par2 & 1) == 0 && ptr->command) {
> + } else if (ptr->command) {
> do_string(ptr->command);
> break;
> } else if (ptr->builtin) {
> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> but I am a little reluctant to do that for 4.2.5 because I haven't figured
> out what case it was supposed to handle. We could make the change in 4.3
> and see if anyone reports problems with x11 or wxt or os2.
This patch also fixes the problem (patch 1 is not applied).
I think the patch 1 should be put to cvs.
I don't know why there is "(par2 & 1) == 0".
---
PM
|
|
From: Ethan M. <merritt@u.washington.edu> - 2009-03-23 23:10:06
|
On Monday 23 March 2009 15:17:33 Petr Mikulik wrote:
> > > > > Can somebody else on Windows reproduce the bug below? I would like this gets
> > > > > fixed before 4.2.5 is released.
> > > > >
> > > > > In Windows, there is only one open and active graph window.
> > > > > I think this could be the reason while mouse.c:event_keypress() reports
> > > > > "protocol error" if you try e.g.:
> > > > >
> > > > > bind 's' 'print "Hello";'
> > > > > bind 'd' 'set grid;'
> > > > > bind 'f' 'set time'
> > > > > bind 'g' 'plot x;'
> > > > > bind 'x' 'test;'
> > > > > plot x*x
> > > > >
> > > > > and then press "s" hotkey (or the others) several times.
> > > > >
> > > > > The code in mouse.c after
> > > > > if (ptr->allwindows && ptr->command) {
> > > > >
> > > > > is testing "allwindows" and "active" windows but it could somehow miss the
> > > > > case of a single-window graph. I'm not sure what should be the correct fix
> > > > > among these several if .. else if ... This well handles case of multiple
> > > > > windows (x11, wxt), but what should it do for single-window cases such as
> > > > > windows or pm terminals?
> > > >
> > > > It must recognize the current window correctly, or it would have exited the
> > > > loop before reaching that protocol error message.
> > > > 1387: } else if (!current) break;
> > > >
> > > > I think it is more likely that some spurious event is being generated
> > > > in addition to the desired keypress event, and we should just ignore it.
> > > > Is the behaviour acceptable if you change the fprintf() to FPRINTF(())?
> > >
> > > Correct behaviour is achieved by setting
> > > ptr->allwindows = 1
> > > or by
> > > par2 = 0
> > > With
> > > current = 1
> > > some events are lost and "protocol error" is shown.
> > >
> > > What (and where) should be set for correct behaviour?
> >
> > OK, with that set of clues I conclude that the failure happens whenever
> > the GE_keypress event notification is sent with a non-zero par2.
> > The only place that can happen is here, and only when
> > thisTimestamp != lastTimestamp. Suggested patch is shown below.
> >
> > %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> >
> > --- gnuplot42/src/win/wgraph.c 2008-10-10 10:21:07.000000000 -0700
> > +++ gnuplot-4.2.5/src/win/wgraph.c 2009-03-19 16:59:30.000000000 -0700
> > @@ -1973,9 +1973,13 @@ Wnd_exec_event(LPGW lpgw, LPARAM lparam,
> > int mx, my;
> > static unsigned long lastTimestamp = 0;
> > unsigned long thisTimestamp = GetMessageTime();
> > + int par2 = thisTimestamp - lastTimestamp;
> > +
> > + if (type == GE_keypress)
> > + par2 = 0;
> >
> > GetMousePosViewport(lpgw, &mx, &my);
> > - gp_exec_event(type, mx, my, par1, thisTimestamp - lastTimestamp, 0);
> > + gp_exec_event(type, mx, my, par1, par2, 0);
> > lastTimestamp = thisTimestamp;
> > }
>
>
> This patch works OK, thanks!
Great. I have added it to cvs. Please check that I didn't mess it up somehow,
since I can't test for a successful Windows build here.
Any other issues to be resolved before pushing out 4.2.5 ?
>
>
> > %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> >
> > I do not know what this Timestamp test is intended to do.
> > No equivalent test exists in the x11 or wxt drivers for keypress events.
> > Perhaps it is intended to handle double-click? If so it should only
> > apply to buttonrelease events, not keypress events.
>
> There are doubleclicks, see "help mouse".
> MB1 copies mouse position to clipboard. This needs to be a doubleclick
> because single click focuses/selects the window.
>
> > It might also be safe to simply remove the test in mouse.c, like this:
> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> > --- gnuplot42/src/mouse.c 2009-02-08 20:43:44.000000000 -0800
> > +++ gnuplot-4.2.5/src/mouse.c 2009-03-19 16:51:07.000000000 -0700
> > @@ -1387,7 +1387,7 @@ event_keypress(struct gp_event_t *ge, TB
> > } else if (!current) {
> > break;
> > /* Let user defined bindings overwrite the builtin bindings */
> > - } else if ((par2 & 1) == 0 && ptr->command) {
> > + } else if (ptr->command) {
> > do_string(ptr->command);
> > break;
> > } else if (ptr->builtin) {
> > %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> > but I am a little reluctant to do that for 4.2.5 because I haven't figured
> > out what case it was supposed to handle. We could make the change in 4.3
> > and see if anyone reports problems with x11 or wxt or os2.
>
> This patch also fixes the problem (patch 1 is not applied).
>
> I think the patch 1 should be put to cvs.
> I don't know why there is "(par2 & 1) == 0".
>
> ---
> PM
>
>
--
Ethan A Merritt
Biomolecular Structure Center
University of Washington, Seattle 98195-7742
|
|
From: Petr M. <mi...@ph...> - 2009-03-24 20:22:09
|
> > > The only place that can happen is here, and only when > > > thisTimestamp != lastTimestamp. Suggested patch is shown below. > > > > > This patch works OK, thanks! > > Great. I have added it to cvs. Please check that I didn't mess it up somehow, > since I can't test for a successful Windows build here. Yes, the patch works OK. > Any other issues to be resolved before pushing out 4.2.5 ? I think it is 4.2.5 can go out. The bug in "screendump" command on Windows will have to wait till the next release (or somebody can make the patch fastly?). --- PM |
