Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jul 19, 2022

When SDL it used on the desktop the SDL include paths are injected
in this way:

$ sdl-config  --cflags
-I/usr/include/SDL -D_GNU_SOURCE=1 -D_REENTRANT
$ sdl2-config  --cflags
-I/usr/include/SDL2 -D_REENTRANT

Its also the way to add include paths for all the other ports.

@sbc100 sbc100 requested a review from dschuff July 19, 2022 15:20
@sbc100 sbc100 enabled auto-merge (squash) July 19, 2022 15:20
Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

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

what's the difference between the 2 sdl-config examples in the commit description?

@sbc100
Copy link
Collaborator Author

sbc100 commented Jul 19, 2022

One is for SDL2 and one is for SDL (v1)

@sbc100 sbc100 force-pushed the sdl_includes branch 3 times, most recently from 2228cf7 to c63cf8f Compare July 20, 2022 01:08
@sbc100
Copy link
Collaborator Author

sbc100 commented Jul 20, 2022

I had to do a little more refactoring to prevent clang from warning about passing -I when building .i or .ii files. PTAL

sbc100 added a commit that referenced this pull request Jul 20, 2022
I noticed that we lacked such a thing while working on #17475.  It turns
out that system header are technically allowed to redefine macros
without generating a warning, but when I made the SDL header tree into a
normal `-I` path I noticed that `M_PI` was being duplicately defined
because we were not defining `HAVE_M_PI`.

This list of defines is taked from `SDL_config_iphoneos.h` and a visual
inspection seems to confirm that we do indeed support all the specified
features.
sbc100 added a commit that referenced this pull request Jul 20, 2022
I noticed that we lacked such a thing while working on #17475.  It turns
out that system header are technically allowed to redefine macros
without generating a warning, but when I made the SDL header tree into a
normal `-I` path I noticed that `M_PI` was being duplicately defined
because we were not defining `HAVE_M_PI`.

This list of defines is taked from `SDL_config_iphoneos.h` and a visual
inspection seems to confirm that we do indeed support all the specified
features.
sbc100 added a commit that referenced this pull request Jul 20, 2022
I noticed that we lacked such a thing while working on #17475.  It turns
out that system header are technically allowed to redefine macros
without generating a warning, but when I made the SDL header tree into a
normal `-I` path I noticed that `M_PI` was being duplicately defined
because we were not defining `HAVE_M_PI`.

This list of defines is taked from `SDL_config_iphoneos.h` and a visual
inspection seems to confirm that we do indeed support all the specified
features.
emcc.py Outdated
flags += ['-mllvm', a]

# relaxed-simd implies simd128.
if '-mrelaxed-simd' in user_args:
Copy link
Member

Choose a reason for hiding this comment

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

unrelated to this change, but what's the difference between flags and user_args?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

user_args is what was supplied on the command line but the user. flags is the flags we are generating to pass to clang.

It does looks like the line below is doing the wrong thing here.. but its not related to this change. I will investigate separately.

sbc100 added a commit that referenced this pull request Jul 21, 2022
I noticed that we lacked such a thing while working on #17475.  It turns
out that system header are technically allowed to redefine macros
without generating a warning, but when I made the SDL header tree into a
normal `-I` path I noticed that `M_PI` was being duplicately defined
because we were not defining `HAVE_M_PI`.

This list of defines is taked from `SDL_config_iphoneos.h` and a visual
inspection seems to confirm that we do indeed support all the specified
features.
sbc100 added a commit that referenced this pull request Jul 21, 2022
I noticed that we lacked such a thing while working on #17475.  It turns
out that system header are technically allowed to redefine macros
without generating a warning, but when I made the SDL header tree into a
normal `-I` path I noticed that `M_PI` was being duplicately defined
because we were not defining `HAVE_M_PI`.

This list of defines is taked from `SDL_config_iphoneos.h` and a visual
inspection seems to confirm that we do indeed support all the specified
features.
sbc100 added a commit that referenced this pull request Jul 21, 2022
I noticed that we lacked such a thing while working on #17475.  It turns
out that system header are technically allowed to redefine macros
without generating a warning, but when I made the SDL header tree into a
normal `-I` path I noticed that `M_PI` was being duplicately defined
because we were not defining `HAVE_M_PI`.

This list of defines is taked from `SDL_config_iphoneos.h` and a visual
inspection seems to confirm that we do indeed support all the specified
features.
sbc100 added a commit that referenced this pull request Jul 22, 2022
I noticed that we lacked such a thing while working on #17475.  It turns
out that system header are technically allowed to redefine macros
without generating a warning, but when I made the SDL header tree into a
normal `-I` path I noticed that `M_PI` was being duplicately defined
because we were not defining `HAVE_M_PI`.

This list of defines is taken from `SDL_config_iphoneos.h` and a visual
inspection seems to confirm that we do indeed support all the specified
features.
When SDL it used on the desktop the SDL include paths are injected
in this way:

```
$ sdl-config  --cflags
-I/usr/include/SDL -D_GNU_SOURCE=1 -D_REENTRANT
$ sdl2-config  --cflags
-I/usr/include/SDL2 -D_REENTRANT
```

Its also the way to add include paths for all the other ports.

As part of this I also refactored the generation of C flags such that
we don't pass `-I` flags when building `.i`/`.ii` files.  These files
are considered to be already pre-processed and clang warns if you pass
`-I` flags when compiling them.
@sbc100 sbc100 merged commit 5d8a6a2 into main Jul 22, 2022
@sbc100 sbc100 deleted the sdl_includes branch July 22, 2022 02:08
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.

3 participants