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

Conversation

@bdero
Copy link
Member

@bdero bdero commented Aug 21, 2023

@bdero bdero requested review from chinmaygarde and zanderso August 21, 2023 05:03
@bdero bdero self-assigned this Aug 21, 2023
@bdero bdero changed the title Add STB config Add testonly STB config Aug 21, 2023

source_root = "//third_party/stb"

source_set("stb_truetype") {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could find a place for this to live in flutter/engine instead of in buildroot?

Copy link
Member

Choose a reason for hiding this comment

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

We could put a tertiary build dir tree that in the engine itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

This would be great. I added an issue about it here: flutter/flutter#132981.

If we move this build config into the engine tree, we should also move the other testonly deps I've added in the past like imgui and tinygltf, as they're in more or less the same situation.


source_root = "//third_party/stb"

source_set("stb_truetype") {
Copy link
Member

Choose a reason for hiding this comment

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

We could put a tertiary build dir tree that in the engine itself.

@chinmaygarde chinmaygarde changed the title Add testonly STB config [Impeller] Add testonly STB config Aug 21, 2023
@bdero bdero merged commit f91786b into flutter:master Aug 21, 2023
@chinmaygarde
Copy link
Member

We discussed this in triage. But build/secondary/third_party/stb/stb_truetype_stub.cc isn't vendored and can live anywhere and doesn't need to live in third_party. I think we can get rid of this and do it entirely in the the engine repo.

@bdero
Copy link
Member Author

bdero commented Aug 22, 2023

But that's what I was trying to address with the bug. Where should we start putting stuff like this in the engine repo? Maybe we could just throw it in as //flutter/third_party/stb:stb_truetype since we need to make the stub TUs anyway?

@chinmaygarde
Copy link
Member

Yeah, this is sort of problematic with libraries that are header only but ask you to stamp out an implementation in exactly on TU by defining a FOO_IMPLEMENATATION macro. We have done this for VMA in flutter_vma.cc. VMA itself is vendored. But the one implmenetation of VMA is in a TU that resides in the engine. And hence its BUILD.gn file can live there too. Perhaps do something similar for STB? Perhaps in the Impeller typographer itself?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants