Thread: [Plib-devel] Minor Issues with PLIB Release Candidate
Brought to you by:
sjbaker
From: Fay J. F C. AAC/W. <joh...@eg...> - 2005-01-14 22:09:12
|
Gentlemen, I'm compiling the PLIB release candidate and also comparing the code with my personal version. There are some points I'd like to raise. It is too bad that I waited this long before looking at it, but I just got back. Anyway, here's the laundry list. (1) There are some compiler warnings under MSVC: - ssgLoadAC.cxx:60 - truncation from const double to float - ssgLoadMDL.cxx:390 - conversion from double to float - ssgSaveIV.cxx:100,233,240,246,255,270 - dynamic-cast used on polymorphic type with "/GR-"; behavior is unpredictable (2) "puAux" is missing a whole lot of changes that I have made in using the library. Some "puAux" widgets refer to deprecated PUI widgets, for example, and some "puaComboBox" behavior fixes that I have made haven't found their way in yet. Another change, which allows the user to activate the widgets with some mouse button other than the left button, also aren't there yet. (3) There is some PUI functionality about when to deactivate the widget that isn't in the release candidate. The arbitrary mouse button change mentioned in connection with "puAux" is also an issue here. Finally, a bug in "puPopupMenu" that puts the subwidgets into the same window as their parent widgets has not been put in either. Are these worth holding up the release? I personally doubt it, although I would push for fixing the compiler warnings. Most users have lived quite happily with the present PUI and "puAux" and that probably won't change. If you do decide to hold up the release, it will be at least until Tuesday because I am about to go home and won't be back until then. John F. Fay joh...@eg... 850-729-6330 |
From: Steve B. <sjb...@ai...> - 2005-01-15 00:08:49
|
Fay John F Contr AAC/WMG wrote: > Gentlemen, > > I'm compiling the PLIB release candidate and also comparing the > code with my personal version. There are some points I'd like to > raise. It is too bad that I waited this long before looking at it... It's not too late. I promised the FGFS guys a release tomorrow *if* nothing fatal came up. We can probably delay a day or two to fix these things. If need be, we can always do another release in a week or two, remember: Version numbers are cheap! > (1) There are some compiler warnings under MSVC: > - ssgLoadAC.cxx:60 - truncation from const double to float > - ssgLoadMDL.cxx:390 - conversion from double to float > - ssgSaveIV.cxx:100,233,240,246,255,270 - dynamic-cast used on > polymorphic type with "/GR-"; behavior is unpredictable I can live with those for now. > (2) "puAux" is missing a whole lot of changes that I have made in using > the library. Some "puAux" widgets refer to deprecated PUI widgets, for > example, and some "puaComboBox" behavior fixes that I have made haven't > found their way in yet. Another change, which allows the user to > activate the widgets with some mouse button other than the left button, > also aren't there yet. Again, those are nice things to have - but not essential to make a release. > (3) There is some PUI functionality about when to deactivate the widget > that isn't in the release candidate. The arbitrary mouse button change > mentioned in connection with "puAux" is also an issue here. Finally, a > bug in "puPopupMenu" that puts the subwidgets into the same window as > their parent widgets has not been put in either. It would be nice to fix that if we can. > Are these worth holding up the release? They are worth holding the release for a day or two - but not for more than that. Dunno if you are able to do this over the weekend. If so, I'll hold up the release - if not - or if I don't hear from you - I'll make the release tomorrow and we'll think in terms of making a second release in a couple of weeks. > I personally doubt it, > although I would push for fixing the compiler warnings. Most users have > lived quite happily with the present PUI and "puAux" and that probably > won't change. If you do decide to hold up the release, it will be at > least until Tuesday because I am about to go home and won't be back > until then. OK. ---------------------------- Steve Baker ------------------------- HomeEmail: <sjb...@ai...> WorkEmail: <sj...@li...> HomePage : http://www.sjbaker.org Projects : http://plib.sf.net http://tuxaqfh.sf.net http://tuxkart.sf.net http://prettypoly.sf.net -----BEGIN GEEK CODE BLOCK----- GCS d-- s:+ a+ C++++$ UL+++$ P--- L++++$ E--- W+++ N o+ K? w--- !O M- V-- PS++ PE- Y-- PGP-- t+ 5 X R+++ tv b++ DI++ D G+ e++ h--(-) r+++ y++++ -----END GEEK CODE BLOCK----- |
From: Bram S. <br...@sa...> - 2005-01-15 07:22:26
|
Fay John F Contr AAC/WMG wrote: > - ssgSaveIV.cxx:100,233,240,246,255,270 - dynamic-cast used on > polymorphic type with "/GR-"; behavior is unpredictable I assume that "/GR-" is a flag for your win32 compiler? What does this flag do? Is removing this flag an option? If not, using isAKindOf() coupled with a static_cast<>() should be a possible replacement of the dynamic cast. I find it strange that win32 is not able to cope with this. I would guess that *all* base classes that have derived classes are polymorphic. That means dynamic_cast cant be used at all in your setup. bram |
From: Frederic B. <fre...@fr...> - 2005-01-15 11:34:27
|
Bram Stolk a =E9crit : > Fay John F Contr AAC/WMG wrote: > >> - ssgSaveIV.cxx:100,233,240,246,255,270 - dynamic-cast used on >> polymorphic type with "/GR-"; behavior is unpredictable > > > I assume that "/GR-" is a flag for your win32 compiler? > What does this flag do? > Is removing this flag an option? > > If not, using isAKindOf() coupled with a static_cast<>() should > be a possible replacement of the dynamic cast. > > I find it strange that win32 is not able to cope with this. > I would guess that *all* base classes that have derived > classes are polymorphic. That means dynamic_cast cant be used > at all in your setup. /GR- is the default option, when you don't specify it in your MSVC=20 project. Somebody should edit ssg.dsp to add the /GR option ( preferably=20 in the IDE, not in vi ). This flag enable C++ RTTI and it is needed=20 because this file use dynamic_cast<>. As rtti introduce some penalty,=20 the flag is not on by default. -Fred |
From: Steve B. <sjb...@ai...> - 2005-01-15 18:14:45
|
Frederic Bouvier wrote: > Bram Stolk a =E9crit : >=20 >> Fay John F Contr AAC/WMG wrote: >> >>> - ssgSaveIV.cxx:100,233,240,246,255,270 - dynamic-cast used on >>> polymorphic type with "/GR-"; behavior is unpredictable >> >> >> >> I assume that "/GR-" is a flag for your win32 compiler? >> What does this flag do? >> Is removing this flag an option? >> >> If not, using isAKindOf() coupled with a static_cast<>() should >> be a possible replacement of the dynamic cast. >> >> I find it strange that win32 is not able to cope with this. >> I would guess that *all* base classes that have derived >> classes are polymorphic. That means dynamic_cast cant be used >> at all in your setup. >=20 >=20 > /GR- is the default option, when you don't specify it in your MSVC=20 > project. Somebody should edit ssg.dsp to add the /GR option ( preferabl= y=20 > in the IDE, not in vi ). This flag enable C++ RTTI and it is needed=20 > because this file use dynamic_cast<>. As rtti introduce some penalty,=20 > the flag is not on by default. It's easier (and faster) to remove the need. I've just committed a version of ssgSaveIV that doesn't use dynamic_cast.= ---------------------------- Steve Baker ------------------------- HomeEmail: <sjb...@ai...> WorkEmail: <sj...@li...> HomePage : http://www.sjbaker.org Projects : http://plib.sf.net http://tuxaqfh.sf.net http://tuxkart.sf.net http://prettypoly.sf.net -----BEGIN GEEK CODE BLOCK----- GCS d-- s:+ a+ C++++$ UL+++$ P--- L++++$ E--- W+++ N o+ K? w--- !O M- V-- PS++ PE- Y-- PGP-- t+ 5 X R+++ tv b++ DI++ D G+ e++ h--(-) r+++ y++++= -----END GEEK CODE BLOCK----- |
From: Steve B. <sjb...@ai...> - 2005-01-15 17:45:42
|
Bram Stolk wrote: > Fay John F Contr AAC/WMG wrote: > >> - ssgSaveIV.cxx:100,233,240,246,255,270 - dynamic-cast used on >> polymorphic type with "/GR-"; behavior is unpredictable > > > I assume that "/GR-" is a flag for your win32 compiler? > What does this flag do? > Is removing this flag an option? > > If not, using isAKindOf() coupled with a static_cast<>() should > be a possible replacement of the dynamic cast. > > I find it strange that win32 is not able to cope with this. > I would guess that *all* base classes that have derived > classes are polymorphic. That means dynamic_cast cant be used > at all in your setup. I don't see why a dynamic cast is needed here. I'm going to dump it and use isAKindOf() and a static cast instead. These relatively new C++ features are always tricky - we should avoid them in software like PLIB where portability is a key feature. ---------------------------- Steve Baker ------------------------- HomeEmail: <sjb...@ai...> WorkEmail: <sj...@li...> HomePage : http://www.sjbaker.org Projects : http://plib.sf.net http://tuxaqfh.sf.net http://tuxkart.sf.net http://prettypoly.sf.net -----BEGIN GEEK CODE BLOCK----- GCS d-- s:+ a+ C++++$ UL+++$ P--- L++++$ E--- W+++ N o+ K? w--- !O M- V-- PS++ PE- Y-- PGP-- t+ 5 X R+++ tv b++ DI++ D G+ e++ h--(-) r+++ y++++ -----END GEEK CODE BLOCK----- |
From: Martin S. <Mar...@un...> - 2005-01-15 21:02:54
|
Steve Baker wrote: > These relatively new C++ features are always tricky - we should > avoid them in software like PLIB where portability is a key feature. While you are talking about portability: Is there any significant reason why the fix that is required to build PLIB on FreeBSD is being silently ignored ? Martin. -- Unix _IS_ user friendly - it's just selective about who its friends are ! -------------------------------------------------------------------------- |
From: Steve B. <sjb...@ai...> - 2005-01-15 21:16:21
|
Martin Spott wrote: > Steve Baker wrote: > > >>These relatively new C++ features are always tricky - we should >>avoid them in software like PLIB where portability is a key feature. > > > While you are talking about portability: Is there any significant > reason why the fix that is required to build PLIB on FreeBSD is being > silently ignored ? I thought all but one of the changes were in there? Certainly the fixes for the joystick library are. The only fix I'm aware of that wasn't committed was the one about removing glIsValidContext. That isn't a bug in PLIB - it's a bug elsewhere. You need to be complaining (or helping with a fix) in some other part of the system rather than breaking this library to cover up a bug in some other library. If glIsValidContext is broken, it's almost certainly just the first symptom of a BUNCH of other things that are broken that you just haven't noticed yet. ---------------------------- Steve Baker ------------------------- HomeEmail: <sjb...@ai...> WorkEmail: <sj...@li...> HomePage : http://www.sjbaker.org Projects : http://plib.sf.net http://tuxaqfh.sf.net http://tuxkart.sf.net http://prettypoly.sf.net -----BEGIN GEEK CODE BLOCK----- GCS d-- s:+ a+ C++++$ UL+++$ P--- L++++$ E--- W+++ N o+ K? w--- !O M- V-- PS++ PE- Y-- PGP-- t+ 5 X R+++ tv b++ DI++ D G+ e++ h--(-) r+++ y++++ -----END GEEK CODE BLOCK----- |
From: Frederic B. <fre...@fr...> - 2005-01-15 21:56:41
|
Steve Baker a =E9crit : > Martin Spott wrote: > >> Steve Baker wrote: >> >> >>> These relatively new C++ features are always tricky - we should >>> avoid them in software like PLIB where portability is a key feature. >> >> >> >> While you are talking about portability: Is there any significant >> reason why the fix that is required to build PLIB on FreeBSD is being >> silently ignored ? > > > I thought all but one of the changes were in there? > > Certainly the fixes for the joystick library are. > > The only fix I'm aware of that wasn't committed was the one about > removing glIsValidContext. That isn't a bug in PLIB - it's a bug > elsewhere. You need to be complaining (or helping with a fix) in > some other part of the system rather than breaking this library to > cover up a bug in some other library. > > If glIsValidContext is broken, it's almost certainly just the first > symptom of a BUNCH of other things that are broken that you just > haven't noticed yet. > I guess Martin was writting about this one : The patch against src/sl/slPortability.h is still missing: --- ./src/sl/slPortability.h.orig Sat Sep 7 17:54:59 2002 +++ ./src/sl/slPortability.h Sat Sep 7 17:55:22 2002 @@ -74,7 +74,7 @@ # if defined(__linux__) # include <linux/soundcard.h> # elif defined(__FreeBSD__) -# include <machine/soundcard.h> +# include <sys/soundcard.h> # else /* Tom thinks this file may be <sys/soundcard.h> under some -Fred |
From: Steve B. <sjb...@ai...> - 2005-01-15 22:23:51
|
Frederic Bouvier wrote: > The patch against src/sl/slPortability.h is still missing: > > --- ./src/sl/slPortability.h.orig Sat Sep 7 17:54:59 2002 > +++ ./src/sl/slPortability.h Sat Sep 7 17:55:22 2002 > @@ -74,7 +74,7 @@ > # if defined(__linux__) > # include <linux/soundcard.h> > # elif defined(__FreeBSD__) > -# include <machine/soundcard.h> > +# include <sys/soundcard.h> > # else > /* > Tom thinks this file may be <sys/soundcard.h> under some Well, someone needs to commit that change. I said several days ago that we were going for a release - it's not like there wasn't any warning. I plan to do another release as soon as John Fay can get his PUI changes in - let's make sure we catch this one next time. ---------------------------- Steve Baker ------------------------- HomeEmail: <sjb...@ai...> WorkEmail: <sj...@li...> HomePage : http://www.sjbaker.org Projects : http://plib.sf.net http://tuxaqfh.sf.net http://tuxkart.sf.net http://prettypoly.sf.net -----BEGIN GEEK CODE BLOCK----- GCS d-- s:+ a+ C++++$ UL+++$ P--- L++++$ E--- W+++ N o+ K? w--- !O M- V-- PS++ PE- Y-- PGP-- t+ 5 X R+++ tv b++ DI++ D G+ e++ h--(-) r+++ y++++ -----END GEEK CODE BLOCK----- |
From: Frederic B. <fre...@fr...> - 2005-01-15 22:40:20
|
Steve Baker wrote : > Frederic Bouvier wrote: > >> The patch against src/sl/slPortability.h is still missing: >> >> --- ./src/sl/slPortability.h.orig Sat Sep 7 17:54:59 2002 >> +++ ./src/sl/slPortability.h Sat Sep 7 17:55:22 2002 >> @@ -74,7 +74,7 @@ >> # if defined(__linux__) >> # include <linux/soundcard.h> >> # elif defined(__FreeBSD__) >> -# include <machine/soundcard.h> >> +# include <sys/soundcard.h> >> # else >> /* >> Tom thinks this file may be <sys/soundcard.h> under some > > > Well, someone needs to commit that change. I said several days > ago that we were going for a release - it's not like there wasn't > any warning. > > I plan to do another release as soon as John Fay can get his > PUI changes in - let's make sure we catch this one next time. The problem here is often that people needing these changes don't have write access and those who have seems to don't bother or just wait someone else do the job. And this project has 24 registered developers :-( -Fred |
From: Martin S. <Mar...@un...> - 2005-01-15 22:49:48
|
Steve Baker wrote: > Well, someone needs to commit that change. I said several days > ago that we were going for a release - it's not like there wasn't > any warning. What else please should I do besides pointing at this missing fix in order to get it committed ? I _did_ post an appropriate comment on this list recently after you anounced you'd go for a release and I already did so some time before. Don't blame me for staying silent, Martin. -- Unix _IS_ user friendly - it's just selective about who its friends are ! -------------------------------------------------------------------------- |
From: Steve B. <sjb...@ai...> - 2005-01-16 05:59:39
|
Martin Spott wrote: > Steve Baker wrote: > > >>Well, someone needs to commit that change. I said several days >>ago that we were going for a release - it's not like there wasn't >>any warning. > > > What else please should I do besides pointing at this missing fix in > order to get it committed ? I _did_ post an appropriate comment on this > list recently after you anounced you'd go for a release and I already > did so some time before. Don't blame me for staying silent, OK - well let's just make sure we get it in *NOW* and get another release out in a week or so. ---------------------------- Steve Baker ------------------------- HomeEmail: <sjb...@ai...> WorkEmail: <sj...@li...> HomePage : http://www.sjbaker.org Projects : http://plib.sf.net http://tuxaqfh.sf.net http://tuxkart.sf.net http://prettypoly.sf.net -----BEGIN GEEK CODE BLOCK----- GCS d-- s:+ a+ C++++$ UL+++$ P--- L++++$ E--- W+++ N o+ K? w--- !O M- V-- PS++ PE- Y-- PGP-- t+ 5 X R+++ tv b++ DI++ D G+ e++ h--(-) r+++ y++++ -----END GEEK CODE BLOCK----- |
From: Steve B. <sjb...@ai...> - 2005-01-16 06:03:42
|
Steve Baker wrote: > Frederic Bouvier wrote: > >> The patch against src/sl/slPortability.h is still missing: >> >> --- ./src/sl/slPortability.h.orig Sat Sep 7 17:54:59 2002 >> +++ ./src/sl/slPortability.h Sat Sep 7 17:55:22 2002 >> @@ -74,7 +74,7 @@ >> # if defined(__linux__) >> # include <linux/soundcard.h> >> # elif defined(__FreeBSD__) >> -# include <machine/soundcard.h> >> +# include <sys/soundcard.h> >> # else >> /* >> Tom thinks this file may be <sys/soundcard.h> under some That change is now committed. ---------------------------- Steve Baker ------------------------- HomeEmail: <sjb...@ai...> WorkEmail: <sj...@li...> HomePage : http://www.sjbaker.org Projects : http://plib.sf.net http://tuxaqfh.sf.net http://tuxkart.sf.net http://prettypoly.sf.net -----BEGIN GEEK CODE BLOCK----- GCS d-- s:+ a+ C++++$ UL+++$ P--- L++++$ E--- W+++ N o+ K? w--- !O M- V-- PS++ PE- Y-- PGP-- t+ 5 X R+++ tv b++ DI++ D G+ e++ h--(-) r+++ y++++ -----END GEEK CODE BLOCK----- |
From: Martin S. <Mar...@un...> - 2005-01-16 07:33:52
|
Steve Baker wrote: > That change is now committed. Thanks, Martin. -- Unix _IS_ user friendly - it's just selective about who its friends are ! -------------------------------------------------------------------------- |