From: Vinnie S. <rcm...@ho...> - 2016-01-19 02:13:07
|
Freeglut has a callback system that is invoked via a macro to remove the need to do type casting, checking if there is a callback to invoke, and making sure the callback window is active so the callback may use other functions on that window. As brought up by Diederick on one of my pull requests (https://github.com/dcnieho/FreeGLUT/pull/42/files#r49944984), there are two of these: https://github.com/dcnieho/FreeGLUT/blob/da7d22e8966bba0bd9e560aab3231bee811 d5c5f/freeglut/freeglut/src/fg_internal.h#L550 One is for Windows and the other is for everything else. My guess is at some point in time, MSVC (Visual Studio compiler) didn't like the format of the code after preprocessing. Deiderick wondered if we can remove the Windows-specific one. I tried compilation in Visual Studio 2015 which succeeded in compiling the general one. Does anyone know which version of MSVC the generic version fails to compile on? If no one has encountered this, can they think of why we should keep it? If not, then we can probably remove it. Note: it was written by fayjf in January of 2012 (yay "git blame"). I don't know why it was written as SVN strangly won't let me see commits before 2014 and since the file was moved prior to having a git mirror (Git supports tracking files through renames and moves, svn does not), I can't go back to 2012 either. The mailing list threads seem to indicate it was John Fay working on making platform specific code. So unless someone wants to dig into the old mailing lists, I can't discern the original reasoning for the code. Vinnie |
From: John T. <nu...@me...> - 2016-01-19 03:01:40
|
On Mon, Jan 18, 2016 at 09:11:50PM -0500, Vinnie Simonetti wrote: > > Does anyone know which version of MSVC the generic version fails > to compile on? If no one has encountered this, can they think of why > we should keep it? If not, then we can probably remove it. > > Note: it was written by fayjf in January of 2012 (yay "git blame"). I think John F. was (is?) using MSVC 6. So I'd start there :) Btw git (and svn) blame won't necessarily reflect the truth. Up to a few years ago all commits where done by John F. as he was the only one active with svn commit access. We used to send him patches in the mailing list and he'd commit them, adding the name of the contributor in the commit logs. Having said that, if it was an ancient MSVC compatibility fix, chances are it was actually John who wrote it :) -- John Tsiombikas http://nuclear.mutantstargoat.com/ |
From: Diederick C. N. <dc...@gm...> - 2016-01-19 07:43:15
|
On Tue, Jan 19, 2016 at 4:01 AM, John Tsiombikas <nu...@me...> wrote: > I think John F. was (is?) using MSVC 6. So I'd start there :) Yes, indeed. Perhaps it is better to use the version with more lines, that would work on all platforms, and the temporary should be optimized away by any compiler with any for of optimization ability. > Btw git (and svn) blame won't necessarily reflect the truth. Up to a few > years ago all commits where done by John F. as he was the only one > active with svn commit access. We used to send him patches in the > mailing list and he'd commit them, adding the name of the contributor in > the commit logs. > Having said that, if it was an ancient MSVC compatibility fix, chances > are it was actually John who wrote it :) Indeed. But when you go back even further (long before i joined), there were a bunch of different people working on the code and comitting themselves. My gut says this code is probably old. I can't get to track the renames for this repo either, but there are a few tags that are of use to drill down (I can do so tonight if needed). https://github.com/dcnieho/FreeGLUT/releases: before_split_platform_code and an even much older before_.... If you check out before_split_platform_code and then do a git blame, you can go back to revision 97 or so! So you'll probably find where the code came from and the associated commit message. Thanks for tracking this down, let me know if you like me to find the commit where this was introduced, can do so in about 12 hours from now. Cheers, Dee |
From: Vinnie S. <rcm...@ho...> - 2016-01-20 02:08:20
|
> From: Diederick C. Niehorster [mailto:dc...@gm...] > Sent: Tuesday, January 19, 2016 2:43 AM > To: FreeGLUT developers list <fre...@li...> > Subject: Re: [Freeglut-developer] INVOKE_WCB Windows vs. non-Windows > > On Tue, Jan 19, 2016 at 4:01 AM, John Tsiombikas <nu...@me...> wrote: > > I think John F. was (is?) using MSVC 6. So I'd start there :) Ah, so that's who I was thinking of. I kept going "someone here uses MSVC 6..." I would've just gone and done variadic macros throughout... but Pre-VS 2005, they aren't supported. I don't know of anyone on VS 2003 .Net, and before that is was MSVC 6. > > Yes, indeed. Perhaps it is better to use the version with more lines, that would work on all platforms, and the temporary should be optimized away by any compiler with any for of optimization ability. > > > Btw git (and svn) blame won't necessarily reflect the truth. Up to a > > few years ago all commits where done by John F. as he was the only one > > active with svn commit access. We used to send him patches in the > > mailing list and he'd commit them, adding the name of the contributor > > in the commit logs. > > Having said that, if it was an ancient MSVC compatibility fix, chances > > are it was actually John who wrote it :) > > Indeed. But when you go back even further (long before i joined), there were a bunch of different people working on the code and comitting themselves. My gut says this code is probably old. I can't get to track the renames for this repo either, but there are a few tags that are of use to drill down (I can do so tonight if needed). > https://github.com/dcnieho/FreeGLUT/releases: > before_split_platform_code and an even much older before_.... If you check out before_split_platform_code and then do a git blame, you can go back to revision 97 or so! So you'll probably find where the code came from and the associated commit message. > > Thanks for tracking this down, let me know if you like me to find the commit where this was introduced, can do so in about 12 hours from now. I'm not sure it's necessary. If we want it gone, then we just need people to actually try to compile it to ensure it still works. > > Cheers, > Dee Vinnie |
From: Diederick C. N. <dc...@gm...> - 2016-01-20 09:18:58
|
Hi Vinnie, On Wed, Jan 20, 2016 at 3:06 AM, Vinnie Simonetti <rcm...@ho...> wrote: >> On Tue, Jan 19, 2016 at 4:01 AM, John Tsiombikas <nu...@me...> > wrote: >> > I think John F. was (is?) using MSVC 6. So I'd start there :) > > Ah, so that's who I was thinking of. I kept going "someone here uses MSVC > 6..." I > would've just gone and done variadic macros throughout... but Pre-VS 2005, > they > aren't supported. I don't know of anyone on VS 2003 .Net, and before that is > was MSVC 6. > >> >> Yes, indeed. Perhaps it is better to use the version with more lines, that > would work on all platforms, and the temporary should be optimized away by > any Reiterating the above. That must compile for John F as its the code path that has been used for a long time when he compiles. So the question is whether its fine for other, non-windows people. I would assume so! I believe you're on a bunch of platforms, try it out. The more lines version is a lot more legible as well, which is a plus. So if you have no problems with it on other platforms (do make sure you see al warnings from the compiler), then its gone in my opinion. Cheers, Dee |
From: John T. <nu...@me...> - 2016-01-20 19:45:48
|
On Wed, Jan 20, 2016 at 10:18:48AM +0100, Diederick C. Niehorster wrote: > > Reiterating the above. That must compile for John F as its the code > path that has been used for a long time when he compiles. So the > question is whether its fine for other, non-windows people. I would > assume so! I believe you're on a bunch of platforms, try it out. The > more lines version is a lot more legible as well, which is a plus. So > if you have no problems with it on other platforms (do make sure you > see al warnings from the compiler), then its gone in my opinion. I agree 100%. There's no point in keeping two versions of the same code, one just slightly more verbose than the other. Btw I checked and it works on linux with gcc (of course it does, why wouldn't it). -- John Tsiombikas http://nuclear.mutantstargoat.com/ |
From: Diederick C. N. <dc...@gm...> - 2016-01-20 19:49:10
|
On Wed, Jan 20, 2016 at 8:45 PM, John Tsiombikas <nu...@me...> wrote: > On Wed, Jan 20, 2016 at 10:18:48AM +0100, Diederick C. Niehorster wrote: >> >> Reiterating the above. That must compile for John F as its the code >> path that has been used for a long time when he compiles. So the >> question is whether its fine for other, non-windows people. I would >> assume so! I believe you're on a bunch of platforms, try it out. The >> more lines version is a lot more legible as well, which is a plus. So >> if you have no problems with it on other platforms (do make sure you >> see al warnings from the compiler), then its gone in my opinion. > > I agree 100%. There's no point in keeping two versions of the same code, > one just slightly more verbose than the other. Btw I checked and it > works on linux with gcc (of course it does, why wouldn't it). Thanks John! Cool, Vinnie, would you like to do the honors and axe the short version?>\ Cheers, Dee |
From: John T. <nu...@me...> - 2016-01-20 19:32:10
|
On Tue, Jan 19, 2016 at 09:06:55PM -0500, Vinnie Simonetti wrote: > > I would've just gone and done variadic macros throughout... but Pre-VS > 2005, they aren't supported. Ehm... variadic macros aren't in standard C89 which we use in freeglut anyway, so ... no :) Also last time I checked, that was a C++11 feature. Did they also add it to C11? Haven't checked. -- John Tsiombikas http://nuclear.mutantstargoat.com/ |
From: Vinnie S. <rcm...@ho...> - 2016-01-21 11:26:13
|
> From: John Tsiombikas [mailto:nu...@me...] > Sent: Wednesday, January 20, 2016 2:32 PM > To: fre...@li... > Subject: Re: [Freeglut-developer] INVOKE_WCB Windows vs. non-Windows > > On Tue, Jan 19, 2016 at 09:06:55PM -0500, Vinnie Simonetti wrote: > > > > I would've just gone and done variadic macros throughout... but Pre-VS > > 2005, they aren't supported. > > Ehm... variadic macros aren't in standard C89 which we use in freeglut anyway, so ... no :) Also last time I checked, that was a C++11 feature. Did they also add it to C11? Haven't checked. I'm well aware. That's why I do compiler checks first before I use anything in C99 or up. I don't remember but I think it is in C11 and C++11. Vinnie |
From: Vinnie S. <rcm...@ho...> - 2016-01-21 11:27:33
|
> > I agree 100%. There's no point in keeping two versions of the same > > code, one just slightly more verbose than the other. Btw I checked and > > it works on linux with gcc (of course it does, why wouldn't it). > > Thanks John! > Cool, Vinnie, would you like to do the honors and axe the short version?>\ Done. It should be in the same pull request with the user callback but I can easily separate it out to a separate pull request if desired. Vinnie |
From: Vinnie S. <rcm...@ho...> - 2016-01-22 02:22:30
|
> From: rcm...@ho... > To: fre...@li... > Date: Thu, 21 Jan 2016 06:26:04 -0500 > Subject: Re: [Freeglut-developer] INVOKE_WCB Windows vs. non-Windows > > > > I agree 100%. There's no point in keeping two versions of the same > > > code, one just slightly more verbose than the other. Btw I checked and > > > it works on linux with gcc (of course it does, why wouldn't it). > > > > Thanks John! > > Cool, Vinnie, would you like to do the honors and axe the short version?>\ > > > Done. It should be in the same pull request with the user callback but I can > easily > separate it out to a separate pull request if desired. > > Vinnie > Also, I did actually find a bug where compiling in c89 mode for GCC would fail becauseit was using the c99 macros. That's fixed now. All others should be fine as they don't appear to support changing which standard theycompile with. So newer compiler=newer "standards". Vinnie |