[RFC] FLTK 1.3.9 (Windows): #define WIN32 in FL/Fl.H

[Albrecht Schlosser]

Hi FLTK devs,

this is pretty late for the FLTK 1.3.x series, but better late than never! Note that I'm planning to release 1.3.9 (soon, maybe parallel to 1.4.0) as the last release in the 1.3.x series.

I believe that it would be helpful to '#define WIN32' in FL/Fl.H to avoid a common user error to "forget" to define 'WIN32' on the commandline. This is documented (maybe too much hidden) but it should be "known". However, new users still seem to forget to do it.
Documentation: https://www.fltk.org/doc-1.3/basics.html#basics_visual_cpp

Note that we use '_WIN32' since FLTK 1.4.0 and we can use this to #define 'WIN32' in 1.3.x as well, w/o the user having to define it.

Please see GitHub Issue #686 for a recent report (with some weird effects).
https://github.com/fltk/fltk/issues/686

I tested the given demo program and I suggest the following fix. Since this may be critical (if done wrong) I'm asking for your comments. It *DOES* fix the issue with the forgotten define (via commandline) in this issue (tested by me).

Proposed fix at the beginning of FL/Fl.H:

// In FLTK 1.3.x WIN32 must be defined on Windows (if not CYGWIN).

// Since FLTK 1.3.9 we define WIN32 if it's not defined on Windows

// to avoid common user errors, for instance GitHub Issue #686.

// Note: since FLTK 1.4.0 we use '_WIN32' anyway, no need to define WIN32.

#if defined(_WIN32) && !defined(__CYGWIN__) && !defined(WIN32)

#define WIN32

#endif

[End of fix]


As patch on branch-1.3:
```

diff --git a/FL/Fl.H b/FL/Fl.H
index 00f589f2b..ea4ceea5c 100644
--- a/FL/Fl.H
+++ b/FL/Fl.H
@@ -23,6 +23,15 @@
 #ifndef Fl_H
 #  define Fl_H

+// In FLTK 1.3.x WIN32 must be defined on Windows (if not CYGWIN).
+// Since FLTK 1.3.9 we define WIN32 if it's not defined on Windows
+// to avoid common user errors, for instance GitHub Issue #686.
+// Note: since FLTK 1.4.0 we use '_WIN32' anyway, no need for WIN32.
+
+#if defined(_WIN32) && !defined(__CYGWIN__) && !defined(WIN32)
+#define WIN32
+#endif
+
 #include <FL/Fl_Export.H>

 #ifdef FLTK_HAVE_CAIRO

```

Questions:

I believe this is safe, but maybe I'm missing something. Comments, anybody?

Should we try to "fix" this at all in 1.3.x which is now end of development?

Personally I think this is a good time to work around such a long standing issue which is not a bug (because it's documented) but it can lead to unexpected behavior (by the user) which can easily be avoided if we #define WIN32 under the given circumstances.

Note: this "works" only sufficiently well (otherwise it may not work) if the user does '#include <FL/Fl.H>' early, i.e. as the first FLTK include but this is documented as well.

What do you think? All comments and suggestions would be appreciated.

[imacarthur]

On Sunday, 26 February 2023 at 18:25:07 UTC Albrecht Schlosser wrote:

Hi FLTK devs,

this is pretty late for the FLTK 1.3.x series, but better late than never! Note that I'm planning to release 1.3.9 (soon, maybe parallel to 1.4.0) as the last release in the 1.3.x series.

I believe that it would be helpful to '#define WIN32' in FL/Fl.H to avoid a common user error to "forget" to define 'WIN32' on the commandline. This is documented (maybe too much hidden) but it should be "known". However, new users still seem to forget to do it.
Documentation: https://www.fltk.org/doc-1.3/basics.html#basics_visual_cpp

Note that we use '_WIN32' since FLTK 1.4.0 and we can use this to #define 'WIN32' in 1.3.x as well, w/o the user having to define it.

[IMM]

Yes - this is pretty much what I have in my code now, to "play nice" with more recent (less ancient) compilers, and it seems to be fine.

 

Please see GitHub Issue #686 for a recent report (with some weird effects).
https://github.com/fltk/fltk/issues/686

I tested the given demo program and I suggest the following fix. Since this may be critical (if done wrong) I'm asking for your comments. It *DOES* fix the issue with the forgotten define (via commandline) in this issue (tested by me).

Proposed fix at the beginning of FL/Fl.H:

// In FLTK 1.3.x WIN32 must be defined on Windows (if not CYGWIN).

// Since FLTK 1.3.9 we define WIN32 if it's not defined on Windows

// to avoid common user errors, for instance GitHub Issue #686.

// Note: since FLTK 1.4.0 we use '_WIN32' anyway, no need to define WIN32.

#if defined(_WIN32) && !defined(__CYGWIN__) && !defined(WIN32)

#define WIN32

#endif

[End of fix]

[IMM]

Is it worth mentioning that many (all?) WIN32 compilers *used* to define WIN32 and (_WIN32 and others of course), but the problem arises because they no longer define WIN32, so apparently breaking much fltk code?

 

Questions:

I believe this is safe, but maybe I'm missing something. Comments, anybody?

[IMM]

I have basically this in a lot of my projects now, and as far as I can tell it works fine with no apparent side effects.

 

Should we try to "fix" this at all in 1.3.x which is now end of development?

Personally I think this is a good time to work around such a long standing issue which is not a bug (because it's documented) but it can lead to unexpected behavior (by the user) which can easily be avoided if we #define WIN32 under the given circumstances.

Note: this "works" only sufficiently well (otherwise it may not work) if the user does '#include <FL/Fl.H>' early, i.e. as the first FLTK include but this is documented as well.

What do you think? All comments and suggestions would be appreciated.

[IMM]

I think it seems sensible, so +1 from me

 

[melcher....@googlemail.com]

Yes, I would put that in.

[Manolo]

Le lundi 27 février 2023 à 09:48:31 UTC+1, melcher....@googlemail.com a écrit :

Yes, I would put that in.

Me too.

[Albrecht Schlosser]

Thanks for all comments (Ian, Matthias, Manolo).

Done in commit 2fddbaea0f67c429585bc241ae99e70a9609e255.