Skip to content

[cmake] mv glad to builtins#22697

Open
ferdymercury wants to merge 1 commit into
root-project:masterfrom
ferdymercury:bglad
Open

[cmake] mv glad to builtins#22697
ferdymercury wants to merge 1 commit into
root-project:masterfrom
ferdymercury:bglad

Conversation

@ferdymercury

Copy link
Copy Markdown
Collaborator

For consistency with other hard-coded builtins such as afterimage and mathtext

@guitargeek

Copy link
Copy Markdown
Contributor

Hi! FYI, glad is not a builtin dependency. It's code auto-generated by the tool with the same name.

Since the code generation was customized to bring exactly what ROOT needs, and the whole idea of glad is that you just copy-paste the generated code into your project, I wouldn't consider this code a "builtin dependency".

@ferdymercury

ferdymercury commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

I wouldn't consider this code a "builtin dependency".

In my opinion: yes and no.
Mathtext is a bit the same: copy-pasted and adapted to ROOT's needs.

Why do I advocate to put in the builtins folder ? Because it was generated externally. This way, if they fix something in the upstream version such as Dav1dde/glad@cef3f89
we can "update" the call to glad and overwrite gl.c files etc. We would only override things in the builtins.

If twice a year we say let's update all our builtins for upstream fixes, we won't forget about glad. If it's deep inside graf3d, we will for sure forget. Getting the overview in a single folder is relevant in my opinion.

@ferdymercury ferdymercury marked this pull request as ready for review June 25, 2026 10:45
@github-actions

Copy link
Copy Markdown

Test Results

    22 files      22 suites   2d 13h 59m 58s ⏱️
 3 874 tests  3 873 ✅ 0 💤 1 ❌
75 383 runs  75 382 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 252d01d.

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.

2 participants