[bgfx] [bimg] [bx] Add new port by BurningEnlightenment · Pull Request #10465 · microsoft/vcpkg

BurningEnlightenment · 2020-03-18T18:24:13Z

Add a port for bgfx. As opposed to #9369 this mostly doesn't use vendored packages, because I replaced the GENIe buildsystem with a straight forward cmake buildsystem with the added benefit that cmake import targets can be provided for everything.

I rewrote the cmake files from scratch and didn't use the ones provided by JoshuaBrookover/bgfx.cmake in order to avoid potential licensing/CLA issues.

However, it would be absolutely awesome if @JoshuaBrookover could review the approach I've taken. Anyways, many thanks for maintaining bgfx.cmake!

The bimg and bgfx ports currently rely on some vendored packages which are currently not available in vcpkg. Also I needed to update some of the existing ports, obviously those updates need to be merged first:

The port has been tested with the x64-windows-static and x64-linux triplets. OSX support is largely lacking due to not having anything to test with.
A "preview" branch with all changes merged can be found here: bgfx-asm

Resolves #1303
Closes #9369

Build the amalgated version of the library using a custom CMake project in order to provide cmake import configs.

Build the library using a custom CMake project in order to provide cmake import configs.

Build the amalgated version of the library using a custom CMake project in order to provide cmake import configs. - Provide feature flags for the most common renderer backends. - Provide feature flags for building the examples and/or tools.

dan-shaw · 2020-03-19T23:07:50Z

/azp run

NancyLi1013 · 2020-03-20T07:44:48Z

ports/bgfx/portfile.cmake

@@ -0,0 +1,82 @@
+
+vcpkg_fail_port_install(ON_TARGET "OSX" "UWP" "ANDROID" ON_LIBRARY_LINKAGE "dynamic")


Since ON_LIBRARY_LINKAGE "dynamic" is not suitable to be used here. ANDROID is not support in vcpkg now.
Please update this as
vcpkg_fail_port_install(ON_TARGET "OSX" "UWP").

If this port only supports static build, you can add vcpkg_check_linkage(ONLY_STATIC_LIBRARY).

If this port only supports static build, you can add vcpkg_check_linkage(ONLY_STATIC_LIBRARY).

Well, the upstream project does support a shared build which exposes the c99 API which currently isn't implemented by this cmake project. Should we still simply fallback to static builds?

We can update this port again if it is implemented by this cmake project.
Currently, we can only support static build.

NancyLi1013 · 2020-03-20T07:48:52Z

ports/bgfx/CONTROL

+Description: Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
+Build-Depends: bx
+Default-Features: opengl, d3d11, tools
+Supports: (windows|linux|static)


This might be updated as Supports: !(uwp|osx).

ports/bgfx/CONTROL

NancyLi1013 · 2020-03-20T08:02:57Z

ports/bgfx/portfile.cmake

+#  * glsl-optimizer  -- https://github.com/aras-p/glsl-optimizer
+#  * iconfontheaders -- https://github.com/juliettef/IconFontCppHeaders
+#  * meshoptimizer   -- https://github.com/zeux/meshoptimizer
+#  * sdf             -- unknown origin


Can these comments be removed here?

I know about the "no comments" guideline, however I'd rather let them stay, because they clearly identify potential port improvements and will help future maintainers implementing these.

ports/bimg/CONTROL

ports/bimg/portfile.cmake

NancyLi1013 · 2020-03-20T08:06:10Z

ports/bimg/portfile.cmake

+#   * etc2          -- unknown source; defines symbols like `::g_table`, high likelyhood of ODR violations
+#   * iqa           -- https://sourceforge.net/projects/iqa or https://github.com/tjdistler/iqa
+#   * nvtt          -- has a vcpkg port which needs to be thoroughly refactored (conflicts with libsquish and others)
+#   * pvrtc         -- https://bitbucket.org/jthlim/pvrtccompressor/


Can these comments be removed here?

ports/bx/portfile.cmake

NancyLi1013 · 2020-03-20T08:09:28Z

/azp run

Upstream bkaradzic/bgfx#2070 recommends using the vendored OpenGL/vulkan headers, because the respective libraries are only dynamically linked at runtime, i.e. ODR violations shouldn't be possible considering the quite minimal C inteface exposed. - Fix some stylistic nits. - Correctly include transitive dependencies.

BurningEnlightenment · 2020-03-21T12:58:16Z

I'm thinking about generally using the vendored dependencies except for those cases where the dependencies are ODR used by installed/exported libraries like lodepng, tinyexr, stb, etc. This would allow us to drop the include patches for cgltf, freetype and glslang without any observable side effects. I will however ask again if the other patches can be integrated upstream

NancyLi1013 · 2020-03-25T09:42:32Z

ports/bgfx/portfile.cmake

@@ -0,0 +1,82 @@
+
+vcpkg_fail_port_install(ON_TARGET "OSX" "UWP" "ANDROID" ON_LIBRARY_LINKAGE "dynamic")


We can update this port again if it is implemented by this cmake project.
Currently, we can only support static build.

NancyLi1013 · 2020-03-26T04:04:39Z

/azp run

NancyLi1013 · 2020-05-12T06:27:34Z

@BurningEnlightenment
Is work still being done for this PR?

NancyLi1013 · 2020-05-14T02:48:15Z

/azp run

azure-pipelines · 2020-05-14T02:48:36Z

Azure Pipelines failed to run 1 pipeline(s).

NancyLi1013 · 2020-05-21T10:30:45Z

@BurningEnlightenment

Thanks for the PR; we're closing this for now since there's been no response. If you'd like to continue working on it, please reopen and ping us!

BurningEnlightenment added 3 commits March 18, 2020 17:24

[bx] Add bx port version af9ccf@2020-03-14

d39406f

Build the amalgated version of the library using a custom CMake project in order to provide cmake import configs.

[bimg] Add bimg port version 2020-03-15

922aeec

Build the library using a custom CMake project in order to provide cmake import configs.

NancyLi1013 assigned Cheney-W and NancyLi1013 Mar 19, 2020

NancyLi1013 suggested changes Mar 20, 2020

View reviewed changes

NancyLi1013 added the waiting for response label Mar 20, 2020

BurningEnlightenment mentioned this pull request Mar 20, 2020

Include path compatibility bkaradzic/bgfx#2070

Closed

BurningEnlightenment added 3 commits March 21, 2020 12:05

[bx] Add cmake transitive dependency discovery

e819f12

[bimg] Add cmake transitive dependency discovery

524bcb2

NancyLi1013 suggested changes Mar 25, 2020

View reviewed changes

NancyLi1013 closed this May 21, 2020

PhoebeHui mentioned this pull request Jan 29, 2021

[New Port Request] bgfx #14955

Closed

		@@ -0,0 +1,82 @@

		vcpkg_fail_port_install(ON_TARGET "OSX" "UWP" "ANDROID" ON_LIBRARY_LINKAGE "dynamic")

Conversation

BurningEnlightenment commented Mar 18, 2020

Uh oh!

dan-shaw commented Mar 19, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NancyLi1013 commented Mar 20, 2020

Uh oh!

BurningEnlightenment commented Mar 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NancyLi1013 commented Mar 26, 2020

Uh oh!

NancyLi1013 commented May 12, 2020

Uh oh!

NancyLi1013 commented May 14, 2020

Uh oh!

azure-pipelines bot commented May 14, 2020

Uh oh!

NancyLi1013 commented May 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BurningEnlightenment commented Mar 21, 2020 •

edited

Loading