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

LibUSB: Update to upstream master (and turn off YAML_CPP_INSTALL CMake property) #8987

Merged
merged 1 commit into from
Oct 16, 2020
Merged

LibUSB: Update to upstream master (and turn off YAML_CPP_INSTALL CMake property) #8987

merged 1 commit into from
Oct 16, 2020

Conversation

bevanweiss
Copy link
Contributor

@bevanweiss bevanweiss commented Sep 27, 2020

May resolve some outstanding issues around libusb/hidapi (i.e. DS4 not being released on rpcs3 shutdown etc..) or may not.
Fixes three outstanding compile warnings on non-Windows OS...
Re-ordered the 3rd party CMake so libusb is in file order above hidapi (just to have document flow match logical flow)

Also took this opportunity to remove the YAML INSTALL action (by setting YAML_CPP_INSTALL to OFF)
Provided capability to use system library for libusb by providing USE_SYS_LIBUSB option to CMAKE
The pkg-config is also used for FreeBSD now to find the dynamic library

Tested under:

  • Windows 10 2004 OS with DS4 controller
  • Windows 10 2004 OS with GCon3 controller
  • Linux (Mint 20) with DS4 controller
  • FreeBSD (GhostBSD) with DS4 controller

Further Testing desired:
- [ ] Other LibUSB devices
- [ ] Testing under 'other platforms' (FreeBSD?)

@bevanweiss
Copy link
Contributor Author

Is the FreeBSD / MacOS testing required / desirable here?
I have tried to get the MacOS build working, if just to test the DS4 lights / vibration under the Pads settings.
And have tried to get FreeBSD build working also, but without success.

If someone already has a FreeBSD / MacOS build working it would be greatly appreciated if they could give this PR a test under those environments (or give me some info on how to get rpcs3 building on either platform).

@hcorion
Copy link
Member

hcorion commented Sep 29, 2020

Since you're changing it to link against upstream, can you also add the ability to link against the system libusb on the nixes? It would be much appreciated towards the goal of #7961 .

@bevanweiss
Copy link
Contributor Author

I'll have a quick look at it tonight. I suspect that we're already using pkg-config elsewhere, so it should be possible to use that to detect the existing libusb and bring in the paths (and libraries) for it...
But I'm not a CMake person, so it might end up in the too hard basket.

For context, our current process has libusb linked into the rpcs3 executable as a static library, and our git already includes the libusb source files.. so where does the 'binary bloat' reduction come in? the git libusb is a spent cost (it will still be a submodule brought to disk), and the rpcs3 executable will still have libusb statically linked into it... so it just seems that we expose ourselves to risk on supporting old versions of libusb (i.e. those bundled with distros), for no benefit. What am I missing?

@hcorion
Copy link
Member

hcorion commented Sep 29, 2020

The idea with allowing linking against dynamic libs is it decreases rpcs3 binary size and time to compile on platforms with a libusb system library. No need to compile and keep around libusb again if it's already there. It's standard practice to use the system's libraries when possible on the unixes.

@bevanweiss
Copy link
Contributor Author

Ahh... so we're talking about dynamic linkage.
I'll have a look tonight, but I suspect it might be 'too hard basket' for me.
Since then we're going to need to statically bring in hidapi, but have both rpcs3 and hidapi bring in libusb as a shared library.

@bevanweiss
Copy link
Contributor Author

bevanweiss commented Sep 30, 2020

@hcorion I think I've got it...
It would be greatly appreciated if you'd be able to test it under your linux system to confirm that it meets the libusb system referencing that you'd expect. I did check with ldd that it was bringing in the system libusb for me, but I'm not a *nix guru, so perhaps I've still done it wrong. I've added an CMAKE_DEPENDENT_OPTION (USE_SYS_LIBUSB) for it, so that the mode can be changed.

…e property)

May resolve some outstanding issues around libusb/hidapi (i.e. DS4 not being released on rpcs3 shutdown etc..) or may not.
Fixes three outstanding compile warnings on non-Windows OS... introduces a few more warnings in WindowsOS (issue raised upstream with libusb around WINAPI_CHECK macro)
Re-ordered the 3rd party CMake so libusb is in file order above hidapi (just to have document flow match logical flow)

Also took this opportunity to remove the YAML INSTALL action (by setting YAML_CPP_INSTALL to OFF)
Provided capability to use system library for libusb by providing USE_SYS_LIBUSB option to CMAKE

Tested under:
- [X] Windows 10 2004 OS with DS4 controller
- [X] Windows 10 2004 OS with GCon3 controller
- [X] Linux (Mint 20) with DS4 controller
@Nekotekina Nekotekina merged commit 1e83d2a into RPCS3:master Oct 16, 2020
@bevanweiss bevanweiss deleted the libusb_update branch October 19, 2020 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants