 |
|
|
|
|
|
| |
| |
|
|
|
|
| |
| |
|
|
In povray-3.50c's unix.cpp, function HandleXEvents(), there is
some code which reads as follows (line 3382):
-----------------------------------------------------------------------
else if (theEvent->xkey.state | ControlMask && (theKeySym == XK_L ||
theKeySym == XK_l || theKeySym == XK_R || theKeySym == XK_r))
{
refresh_y_min = 0;
refresh_y_max = Frame.Screen_Height;
}
-----------------------------------------------------------------------
This is a bug. It must be:
-----------------------------------------------------------------------
else if ((theEvent->xkey.state & ControlMask) && (theKeySym == XK_L ||
theKeySym == XK_l || theKeySym == XK_R || theKeySym == XK_r))
{
refresh_y_min = 0;
refresh_y_max = Frame.Screen_Height;
}
-----------------------------------------------------------------------
Notice the '|' before ControlMask; put brackets around it for clarity.
Please fix that in 3.51.
Wolfgang
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
> This is a bug. It must be:
Can you clarify the nature of the problem you're fixing ?
- NC
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Nicolas Calimet wrote:
>> This is a bug. It must be:
>
> Can you clarify the nature of the problem you're fixing ?
>
Sorry, but the presence of a flag is tested by using
state & flag
and not
state | flag
because the latter is always true.
Hence, writing
is a bug and it must be
just from reading the code.
So, in order to comply with the description saying that Control-L
does a complete screen re-draw (as usual in Unix), one has to
apply the change I mentioned.
I was not aware that any clarification was needed for that...
...or what is the problem?
Wolfgang
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Wolfgang Wieser wrote:
> Nicolas Calimet wrote:
>
>>>This is a bug. It must be:
>>
>>Can you clarify the nature of the problem you're fixing ?
>>
>
> Sorry, but the presence of a flag is tested by using
> state & flag
> and not
> state | flag
> because the latter is always true.
Does the phrase "if it ain't broken, don't fix it" mean anything to you?
I think Nicolas was asking for and explanation why to apply this change.
Something like 'in case of the xxx situation POV-Ray does yyy which is
not correct'.
Christoph
--
POV-Ray tutorials, include files, Sim-POV,
HCR-Edit and more: http://www.tu-bs.de/~y0013390/
Last updated 25 Oct. 2003 _____./\/^>_*_<^\/\.______
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
> Sorry, but the presence of a flag is tested by using
> state & flag
> and not
> state | flag
> because the latter is always true.
No. The latter is false when both state and flag are false.
It's true in any other case.
Maybe it's always true in the context of the povray code
you report, but honestly I have no idea what can be the values
of theEvent->xkey.state and ControlMask.
> I was not aware that any clarification was needed for that...
> ...or what is the problem?
When fixing a problem, you'd better give a description
of the problem _first_ :-)
I suppose you found that CTRL+L was not refreshing the
screen as expected. If your fix does solve the problem, then
it will be included in the next povray version after testing.
- NC
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Christoph Hormann wrote:
> Wolfgang Wieser wrote:
>> Nicolas Calimet wrote:
>>
>>>>This is a bug. It must be:
>>>
>>>Can you clarify the nature of the problem you're fixing ?
>>>
>>
>> Sorry, but the presence of a flag is tested by using
>> state & flag
>> and not
>> state | flag
>> because the latter is always true.
>
> Does the phrase "if it ain't broken, don't fix it" mean anything to you?
>
This phrase does not reflect my philisophy.
And -- if you read code like that:
if(state | StateBit) { do sth }
What would you think about the author? -- Oh dear...
> I think Nicolas was asking for and explanation why to apply this change.
>
To ask for "the nature of the problem" sounded to me like complete
lack of understanding... like a "tell me what's going on here?!".
I was really puzzled -- I thought for a UNIX guy like Nicolas
the point should be fairly clear.
> Something like 'in case of the xxx situation POV-Ray does yyy which is
> not correct'.
>
I provided the answer, IMO -- quoting myself:
So, in order to comply with the description saying that Control-L
does a complete screen re-draw (as usual in Unix), one has to
apply the change I mentioned.
Wolfgang
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Nicolas Calimet wrote:
>> Sorry, but the presence of a flag is tested by using
>> state & flag
>> and not
>> state | flag
>> because the latter is always true.
>
> No. The latter is false when both state and flag are false.
> It's true in any other case.
>
Oh dear... I was doing raw Xlib programming, so thinks like
these are familiar to me:
bash# fgrep -r ControlMask /usr/include/X11/*.h
/usr/include/X11/X.h:#define ControlMask (1<<2)
> Maybe it's always true in the context of the povray code
> you report, but honestly I have no idea what can be the values
> of theEvent->xkey.state and ControlMask.
>
It's simple: When the control key is pressed, the ControlMask
bit is set on the state value. Otherwise not. To test for
pressed control key, one checks for if(state & ControlMask)
>> I was not aware that any clarification was needed for that...
>> ...or what is the problem?
>
> When fixing a problem, you'd better give a description
> of the problem _first_ :-)
>
Yeah -- and I thought everyone would see the point after looking
just 2 seconds at the code. It was just soooo evident to me.
Wolfgang
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Wolfgang Wieser wrote:
>
> Yeah -- and I thought everyone would see the point after looking
> just 2 seconds at the code. It was just soooo evident to me.
>
Please understand that also for a user interface bug the basic rules of
bug reporting apply, i.e. clear description of the symptoms of the
problem is required. If you had said you tested that this change fixes
a lack of response to ctrl-l in POV-Ray everything would have been clear.
Christoph
--
POV-Ray tutorials, include files, Sim-POV,
HCR-Edit and more: http://www.tu-bs.de/~y0013390/
Last updated 25 Oct. 2003 _____./\/^>_*_<^\/\.______
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
> bash# fgrep -r ControlMask /usr/include/X11/*.h
> /usr/include/X11/X.h:#define ControlMask (1<<2)
Okay, in this case I agree it's always true :-)
> Yeah -- and I thought everyone would see the point after looking
> just 2 seconds at the code. It was just soooo evident to me.
Sure, and I actually got the point when reading the code
(but was still too lazy to grep for ControlMask). My first post
was simply intended to pass the message that giving a clear & simple
description of the problem may help saving time. However, this
message was not clear either... ;-)
Anyway, thanx for fiding this. The Xwindows code would
actually need more extensive review, so if you find any other
problem which is not related to Xwindows-specific command-line
options (this part of the code is quite buggy; currently fixed),
you're welcome to report !
- NC
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
In article <3fc3c9ad@news.povray.org> , Wolfgang Wieser <wwi### [at] gmx de>
wrote:
>>> Sorry, but the presence of a flag is tested by using
>>> state & flag
>>> and not
>>> state | flag
>>> because the latter is always true.
>>
>> Does the phrase "if it ain't broken, don't fix it" mean anything to you?
>>
> This phrase does not reflect my philisophy.
>
> And -- if you read code like that:
> if(state | StateBit) { do sth }
> What would you think about the author? -- Oh dear...
Compared to code used in some commercial applications, the code in POV-Ray
is almost perfect. ;-)
Sadly, for any project that is big or complex enough you cannot say much
about the authors if you just look at the code when it is 15 years old and
has been treated by dozens of people. Even if it would be a new project, at
best you can say how good the project management was, but still very little
about the authors.
Saying something about one particular author requires to make sure only one
author ever touched the code. If you are lucky you find such condition in
an entry-level programming course, not anywhere else :-(
Thorsten
____________________________________________________
Thorsten Froehlich, Duisburg, Germany
e-mail: tho### [at] trfde
Visit POV-Ray on the web: http://mac.povray.org
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
