Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "FGAPI" before all core instanciation of Interface functions of Freeglut v3.4.0 #159

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tdechaize
Copy link

In addition of my tests of build Freeglut v3.4.0 on Windows, I constat that Pelles C Compiler is most very strict than all others compilers tested (CLANG,MINGW, MSVC, DMC, Borland C, etc...). It impose that all core instanciation of Interface functions of FG be preceed by FGAPI like declaration in "include\GL\freeglut_std.h".
For all others compilers tested this evolution is totally transparent, I realize this operation on all sources concerned of FG.
Thank's.
Thierry D.

After succes build with many compilers on Windows 11, these modifications are mandatory.
To resume major evolutions in source code :

    a) in CMakeLists.txt, adapt environment to win the build with CLANG, Borland C/C++, Open WATCOM and above all CYGWIN, because you can't build Freeglut with CYGWIN/X11 (library Xxf86vm inexistent),

    For CYGWIN, I can build Freeglut with MinGW32 or MinGW64 configured in CYGWIN with success.

   b) still with CMakeLists.txt, I propose to distinguish "Debug" build with postfix "d" for all platforms (and "Release" build with no postfix), and switch beetween  two mode with variable CMAKE_BUILD_TYPE

   c) On Windows, only Pelles C compiler don't compile if instanciation of all interface functions of Freeglut is "syntaxilly" different of declaration in include file.

   On all initial "operationnal" core sources of Freeglut , these function are not prefixed by FGAPI, but in include file freeglut_std.h this prefix exist ..., and Pelles C refuse compilation.

   You can add FGAPI to all interface function in core source, all compilers,  less strict than Pelles C, accept this change. I do the change, and realize success build with all compilers on Windows, like Pelles C.
This adding is mandatory for Pelles C Compiler, and is totally transparent for all compilers tested on Windows 11.
Copy link
Member

@jtsiomb jtsiomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started writing detailed comments, but it quickly became apparent that it's pointless. This is also a bunch of random things thrown together, not just the FGAPI fixes as your title suggests.

The CMakeLists.txt changes are a huge mess with grammatical mistakes in error messages, weird absolute paths, and a wall of text of comments. This is unacceptable.

For the rest of the changes, beyond the fact that you need to move anything other than FGAPI additions to separate pull requests:

  • drop unnecessary comments explaining the rationale for changes
  • drop dates of changes from comments
  • make sure to only use C89-style /* .. */ comments
  • drop unnecessary formatting and whitespace changes.

@jtsiomb
Copy link
Member

jtsiomb commented Apr 18, 2024

Also, some of your changes are adding extra conditions to HAVE_WHATEVER_H for specific compilers. I feel like this is probably an issue in your machine. Why would cmake find the file, but the compiler fail to do so? Before you resubmit this kind of workarounds, try to reproduce the problem on a clean VM with only that specific compiler installed.

@tdechaize
Copy link
Author

Hi,
Yes, you rigth, and forgive me. I mix two versions on two different directories of very important file : CMakeLists.txt on last commit, with use of GitHub Desktop (newbie fault ... !!).
Today, I renew this commit whith many verifications.
One add don't concern new prefix FGAPI in code interface functions of Freeglut version 3.4.0 : in file "freeglut_std.h", I conserve test on additional compilers on Windows about #define (declaration) of "bitmap" and "stroke" ident of characters set in this file (line 216). All tested with success.
I think that this add has not impact on rest of generation of FG (added ... || defined(BORLANDC) || defined(DMC) || defined(LCC) || defined(POCC))
Sorry for last commit, I learn about my errors ...
Thank's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants