Skip to content

Commit

Permalink
bgfx: make bx and bimg private
Browse files Browse the repository at this point in the history 
This makes it so in shared mode, bx and bimg static libs aren't installed
  • Loading branch information
@bwrsandman
bwrsandman committed Apr 29, 2021
1 parent 2bd5c20 commit f8f30ca
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion cmake/bgfx.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ target_include_directories( bgfx
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>)

# bgfx depends on bx and bimg
target_link_libraries( bgfx PUBLIC bx bimg )
target_link_libraries( bgfx PRIVATE bx bimg )

# ovr support
if( BGFX_USE_OVR )
Expand Down
2 changes: 1 addition & 1 deletion cmake/examples.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ function( add_example ARG_NAME )
if( ARG_COMMON )
add_library( example-${ARG_NAME} STATIC EXCLUDE_FROM_ALL ${SOURCES} )
target_include_directories( example-${ARG_NAME} PUBLIC ${BGFX_DIR}/examples/common )
target_link_libraries( example-${ARG_NAME} PUBLIC bgfx dear-imgui meshoptimizer )
target_link_libraries( example-${ARG_NAME} PUBLIC bgfx bx bimg dear-imgui meshoptimizer )
if( BGFX_WITH_GLFW )
find_package( glfw3 REQUIRED )
target_link_libraries( example-${ARG_NAME} PUBLIC glfw )
Expand Down

3 comments on commit f8f30ca

@pr0g
Copy link

@pr0g pr0g commented on f8f30ca Jun 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @bwrsandman,

I ran into an issue with this change causing a build break and wanted to check if this change was the right thing to do.

Right now in bx.cmake there's some code to setup the include directories for bx to use. One is the compat files (for things like alloca.h on Windows). By making bx private, that include directory is no longer transitively brought in by bgfx, so the consuming library must also explicitly depend on bx which can be confusing... (see this commit - pr0g/sdl-bgfx-imgui-starter@068dc2c).

For example embedded_shader.h (in the include folder of bgfx) contains #include <bx/bx.h>, which constitues part of the public API of bgfx, so bx can't (yet at least) be a private dependency.

It seems like it's safe to make bimg private but could bx please me made public again (until all references to bx.h have been moved out of bgfx.h headers).

Thanks for your time,

Tom

@bwrsandman
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

An issue comes up for installations on *nix systems that those same headers can create conflicts with existing headers in /usr/include, for example having tinystl installed. Not to mention /usr/include/compat is very ambiguous.

We will have to think of a solution for this.

@pr0g
Copy link

@pr0g pr0g commented on f8f30ca Jun 30, 2021

Choose a reason for hiding this comment

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

Thanks for the context @bwrsandman.

Ah I see... I usually install to a custom directory under a bgfx folder so haven't run into this but I can see how that could interfere with other headers on *nix systems. If everything was under include/bx/compat it might not be so bad? But yeah tricky to get right across all platforms without breaking anything...

I'm unblocked for now but I just think it's one of those things that could be easy to trip up on and make usage more difficult for new users.

Thanks for getting back to me so quickly.

Tom

Please sign in to comment.