Skip to content

Fix SDL2 header access issue #316

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

Merged
merged 1 commit into from
May 27, 2025

Conversation

anutosh491
Copy link
Collaborator

Description

Fix #301

Type of change

Please tick all options which are relevant.

  • Bug fix
  • New feature
  • Added/removed dependencies
  • Required documentation updates

@anutosh491
Copy link
Collaborator Author

anutosh491 commented May 27, 2025

Hi,

We had merged the PR adding the smallpt example based on SDL2 and it worked very smoothly when done locally. This is because it wasn't the first time I was building xeus-cpp-lite with the sdl2 enabling flag and everything works perfectly the second time on.

Why is that because we have this

        PUBLIC "SHELL: -s USE_SDL=2"
        PUBLIC "SHELL: --preload-file ${SYSROOT_PATH}/include@/include"

Now here USE_SDL2 does fetch the sdl2 ports ( sdl2 files/libs aren't provided by emsdk by default, they need to be fetched)
Now although the above flag would fetch it, when we preload the sysroot's include directory at runtime ... the SDL2 headers won't be present in include just yet (and probably would be present once this process finishes .... which explain why building the second time does the job).

Hence we need to make sure we fetch the ports even before the link step (so that include has access to SDL2 being loaded and at linktime, the SDL2 lib i.e libSDL2 is already present)

Many ways to do this

  1. create an empty file and do emcc -sUSE_SDL2 dummy.c before building xeus-cpp
  2. Use embuilder that emscripten recommends for building the 3rd party libs they support.

image

anutosh491@Anutoshs-MacBook-Air xeus-cpp % embuilder --help
usage: embuilder.py [-h] [--lto] [--lto=thin] [--pic] [--force] [--verbose] [--wasm64] {build,clear,rebuild} [targets ...]

Tool to manage building of system libraries and ports.

In general emcc will build them automatically on demand, so you do not
strictly need to use this tool, but it gives you more control over the
process (in particular, if emcc does this automatically, and you are
running multiple build commands in parallel, confusion can occur).

positional arguments:
  {build,clear,rebuild}
  targets               see below

Available targets:

  build / clear
        boost_headers
        bullet
        bzip2
  ........
        sdl2
.......
        sqlite3
        sqlite3-mt
        vorbis
        zlib

I've used approach 2 to fetch the sdl2 ports which needs to be done before any preloading.

As can be seen here we fetch sdl2 ports successfully even before building.

https://github.com/compiler-research/xeus-cpp/actions/runs/15271106052/job/42946728657?pr=316#step:5:123

@anutosh491
Copy link
Collaborator Author

cc @SylvainCorlay

We could have obviously cut down to just SDL which is provided by default but I think its good to provide SDL2 access to the users.

@codecov-commenter
Copy link

codecov-commenter commented May 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.78%. Comparing base (aace669) to head (00f2db6).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #316   +/-   ##
=======================================
  Coverage   81.78%   81.78%           
=======================================
  Files          20       20           
  Lines         950      950           
  Branches       87       87           
=======================================
  Hits          777      777           
  Misses        173      173           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@IsabelParedes IsabelParedes left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

LGTM...

@anutosh491 anutosh491 merged commit 7817031 into compiler-research:main May 27, 2025
14 checks passed
@anutosh491 anutosh491 deleted the fix_sdl2_issue branch May 27, 2025 12:22
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.

Bug while running smallpt example notebook with lite
4 participants