Skip to content
This repository was archived by the owner on Apr 21, 2025. It is now read-only.

Add glad target to glfw/BUILD.gn. #275

Closed
wants to merge 2 commits into from

Conversation

cloudwebrtc
Copy link

Used to solve flutter/engine#9822.

@stuartmorgan-g
Copy link
Contributor

Having thought about this more since my earlier comments, I don't think this is the approach we should take for a glad dependency. It's a totally separate project from GLFW, and only happens to be in the GLFW repository because some example app in the GLFW tree uses it (IIRC). And the use you're adding isn't specific to GLFW either.

Given all that I think glad should be added as a new third_party directory in the Flutter repository, with its own build file, license file, updating instructions, etc.

@cloudwebrtc
Copy link
Author

@stuartmorgan I made some changes. Should I submit the glad directory directly to the flutter/engine/third_party directory instead of buildroot/third_party?

I see that the third_party directory in flutter/engine has only one txt directory. Is this the right way to store third-party libraries?

@stuartmorgan-g
Copy link
Contributor

Should I submit the glad directory directly to the flutter/engine/third_party directory instead of buildroot/third_party?

I believe so, yes.

@cbracken Am I remembering correctly that only DEPS-managed things go in flutter/buildroot/third_party, while new directly-checked-in code goes in flutter/engine/third_party?

@cbracken
Copy link
Member

@cbracken Am I remembering correctly that only DEPS-managed things go in flutter/buildroot/third_party, while new directly-checked-in code goes in flutter/engine/third_party?

Correct -- for anything managed as a simple pull from upstream via the DEPS file is pulled to src/third_party, whereas library code that we plan to check in and patch/develop ourselves (e.g. libtxt) goes in src/flutter/third_party. Currently, libtxt is the only such code and we develop it entirely on our own. Generally we want to avoid adding any further self-developed dependencies like this.

Just to get a better understanding of what's being proposed here -- can this be pulled down via DEPS like other dependencies (rather than checking in source files)? If this is generated code, can we not run a script to generate this from a tool pulled down via DEPS?

@cbracken
Copy link
Member

@cloudwebrtc friendly ping.

@stuartmorgan-g
Copy link
Contributor

If this is generated code, can we not run a script to generate this from a tool pulled down via DEPS?

Based on the project's readme, it does look like this should be possible. We'd pull it via DEPS (or check it in, depending on mirroring requirements) and have a GN rule that generates the header from their python script.

@cloudwebrtc Is that something you'd be interested in taking on?

@cloudwebrtc
Copy link
Author

@stuartmorgan
So can I merge the glad patch directly into texture for glfw? Because it seems that only texture for glfw uses the glad library.

@stuartmorgan-g
Copy link
Contributor

I think it would be useful to keep it as a separate patch, to keep it simpler to review. It'll have its own build logic, license updates, etc., which would be harder to review as part of the other patch.

@cloudwebrtc
Copy link
Author

Thanks, I will create a separate patch for flutter/engine, so close this issue now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants