Skip to content

libsdl3: Added extra configuration options#9775

Open
rempas wants to merge 2 commits intoxmake-io:devfrom
rempas:rempas-patch-1
Open

libsdl3: Added extra configuration options#9775
rempas wants to merge 2 commits intoxmake-io:devfrom
rempas:rempas-patch-1

Conversation

@rempas
Copy link
Copy Markdown
Contributor

@rempas rempas commented Apr 17, 2026

This patch adds extra configuration options to allow to disable parts of sdl3 that we don't need.

  • Before adding new features and new modules, please go to issues to submit the relevant feature description first.
  • Write good commit messages and use the same coding conventions as the rest of the project.
  • Please commit code to dev branch and we will merge into master branch in feature
  • Ensure your edited codes with four spaces instead of TAB.

  • 增加新特性和新模块之前,请先到issues提交相关特性说明,经过讨论评估确认后,再进行相应的代码提交,避免做无用工作。
  • 编写友好可读的提交信息,并使用与工程代码相同的代码规范,代码请用4个空格字符代替tab缩进。
  • 请提交代码到dev分支,如果通过,我们会在特定时间合并到master分支上。
  • 为了规范化提交日志的格式,commit消息,不要用中文,请用英文描述。

This patch adds extra configuration options to allow to disable parts of sdl3 that we don't need.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces granular configuration options for libsdl3, allowing users to toggle specific features like video, audio, and GPU support. It refactors the package's dependency logic to conditionally include headers and libraries based on these configurations. The review feedback identifies typos in the configuration descriptions and highlights necessary adjustments for platform-specific dependencies, specifically regarding wasm and X11.

Comment thread packages/l/libsdl3/xmake.lua Outdated
add_configs("haptic", { description = "Enable haptic input support", default = true, type = "boolean"})
add_configs("camera", { description = "Enable support for SDL_Camera", default = true, type = "boolean"})
add_configs("storage", { description = "Enable support for SDL_Storage", default = true, type = "boolean"})
add_configs("process", { description = "Enable support for SDL's cross-platform process spawing", default = true, type = "boolean"})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Typo in description: 'spawing' should be 'spawning'.

    add_configs("process", { description = "Enable support for SDL's cross-platform process spawning", default = true, type = "boolean"})

Comment thread packages/l/libsdl3/xmake.lua Outdated
add_configs("process", { description = "Enable support for SDL's cross-platform process spawing", default = true, type = "boolean"})
add_configs("dialog", { description = "Enable support for SDL's native file/directory picker and dialog", default = true, type = "boolean"})
add_configs("tray", { description = "Enable support for SDL's tray system API", default = true, type = "boolean"})
add_configs("filesystem", { description = "Enable support for SDL's standar file path handling", default = true, type = "boolean"})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Typo in description: 'standar' should be 'standard'.

    add_configs("filesystem", { description = "Enable support for SDL's standard file path handling", default = true, type = "boolean"})

Comment thread packages/l/libsdl3/xmake.lua Outdated
on_load(function (package)
local supports_video = package:config("video")

if supports_video then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The egl-headers and opengl-headers dependencies should not be added for the wasm platform, as they are explicitly skipped in the on_install block (line 170). This ensures consistency between the package dependencies and the build configuration.

        if supports_video and not package:is_plat("wasm") then

Comment thread packages/l/libsdl3/xmake.lua Outdated
end
if package:is_plat("linux", "bsd", "cross") and package:config("x11") then
if package:is_plat("linux", "bsd", "cross") and package:config("x11") and supports_video then
local deplibs = {"libx11", "libxcb", "libxext", "libxcursor", "libxfixes", "libxi", "libxrandr", "libxrender", "libxss"}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The xorgproto dependency is missing from the deplibs list in on_load. It must be added as a direct dependency here so that package:dep("xorgproto") can successfully retrieve its include directories in the on_install block (line 191).

            local deplibs = {"libx11", "libxcb", "libxext", "libxcursor", "libxfixes", "libxi", "libxrandr", "libxrender", "libxss", "xorgproto"}

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.

1 participant