Skip to content
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

[tracy] compatible with non wayland systems #31520

Merged
merged 1 commit into from
May 25, 2023

Conversation

fran6co
Copy link
Contributor

@fran6co fran6co commented May 19, 2023

Tracy 0.9.1 makes Wayland the default backend for the gui tool but a lot of systems don't support it yet, switching back to glfw3 keeps it backward compatible

@BillyONeal BillyONeal changed the title vcpkg - make it compatible with non wayland systems [tracy] compatible with non wayland systems May 20, 2023
@BillyONeal
Copy link
Member

This seems to reverse a decision upstream explicitly made, so I think you should be talking to them first...

@fran6co
Copy link
Contributor Author

fran6co commented May 20, 2023 via email

@Cheney-W Cheney-W added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label May 22, 2023
@Cheney-W
Copy link
Contributor

In general, it is advisable to follow the upstream design and seek their approval for any modifications made to the source code. Therefore, could you please communicate the current situation to the upstream project and seek their feedback? Thank you!

@Cheney-W Cheney-W marked this pull request as draft May 22, 2023 10:35
@fran6co
Copy link
Contributor Author

fran6co commented May 22, 2023

But it's not a modification to the source code, it's just passing the option to target the old backed that works for more windowing systems than just wayland. GLFW works for wayland and x11

@fran6co
Copy link
Contributor Author

fran6co commented May 22, 2023

The modification to the code is the same vcpkg had for 0.9.0 before the upstream change just to make it use vcpkg imgui instead of the repo imgui

@fran6co
Copy link
Contributor Author

fran6co commented May 22, 2023

To make it easier I just created gui-tools-legacy that builds the gui-tools with the legacy flag

@fran6co fran6co marked this pull request as ready for review May 22, 2023 11:04
@dg0yt
Copy link
Contributor

dg0yt commented May 22, 2023

GLFW works for wayland and x11

To make it easier I just created gui-tools-legacy that builds the gui-tools with the legacy flag

Just to verify: with this feature selected, the package supports x11 in addition to wayland, using a different implementation under the hood?

@fran6co
Copy link
Contributor Author

fran6co commented May 22, 2023 via email

Cheney-W
Cheney-W previously approved these changes May 23, 2023
@Cheney-W
Copy link
Contributor

All features test passed with below triplets:
x64-windows
x64-windows-static

@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label May 23, 2023
@dan-shaw
Copy link
Contributor

This is also against our features should not be alternatives policy, so I think we would recommend users to use port overlays in this case.

@fran6co
Copy link
Contributor Author

fran6co commented May 24, 2023

That's why originally I just made glfw backend the default as it offers more compatibility. Upstream still offers as an option I don't understand why vcpkg can't make it the default

@dg0yt
Copy link
Contributor

dg0yt commented May 25, 2023

This is also against our features should not be alternatives policy, so I think we would recommend users to use port overlays in this case.

This should be the reason why the legacy choice should be used in general for now: It supports new and old systems without needing overlays AFAIU. And this was originally proposed.

@fran6co
Copy link
Contributor Author

fran6co commented May 25, 2023

Changed back to the original proposal

@Cheney-W
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dan-shaw dan-shaw merged commit 24a3b63 into microsoft:master May 25, 2023
petersj-ess added a commit to enterprise-software-systems/vcpkg that referenced this pull request Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants