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

add device_type inference to gl backend #2595

Merged
merged 2 commits into from Jan 24, 2019

Conversation

Projects
None yet
3 participants
@jackmott
Copy link
Contributor

jackmott commented Jan 23, 2019

A first pass at inferring the device type. If we see "intel" in the device string we can be confident that it is an integratedgpu. If we see "nvidia", "ati", or "amd" it is either going to be discrete or have performance much better than the intel integrated gpu. Otherwise we set device_type to Other since we really have no idea.

Fixes #2593
PR checklist:

  • make succeeds (on *nix)
  • make reftests succeeds
  • tested examples with the following backends:gl
  • rustfmt run on changed code
@jackmott

This comment has been minimized.

Copy link
Contributor

jackmott commented Jan 23, 2019

Maybe this whole idea is no good. Intel has discrete cards coming:
https://www.tomshardware.com/news/intel-discrete-gpu-2020-raja,37289.html

Should we always say Other? Should we add an Unknown to the Device_Type enum? This would break from the Vulkan API though.

@kvark

This comment has been minimized.

Copy link
Member

kvark commented Jan 24, 2019

Note that AMD also has APUs, which are integrated... I wonder we can do a more precise matching of the adapter name to figure this out. As for the current PR, I don't feel strongly either way. Anyone got a strong opinion here?

@grovesNL

This comment has been minimized.

Copy link
Member

grovesNL commented Jan 24, 2019

Not really, I'd slightly prefer a stronger heuristic than vendor.

@jackmott

This comment has been minimized.

Copy link
Contributor

jackmott commented Jan 24, 2019

I'm going to do some research on vendor/version/renderer strings and see if we can do better. I've got quite a few computers here to survey.

@jackmott

This comment has been minimized.

Copy link
Contributor

jackmott commented Jan 24, 2019

I've got a simple command line program that will print renderer vender and version:
https://github.com/jackmott/rustglstats

I've started compiling results for computers I have access to:
https://docs.google.com/spreadsheets/d/1va19XlGv9VuUEF99ekkWhDoTroYOp-QRAIU457lAnM4/edit?usp=sharing

if any of you want to add to it or pass this around please do

@kvark

This comment has been minimized.

Copy link
Member

kvark commented Jan 24, 2019

Could also use glxinfo that is included by default in Linux. There is also a database in https://opengl.gpuinfo.org/

@jackmott

This comment has been minimized.

Copy link
Contributor

jackmott commented Jan 24, 2019

I figured something like that would exist already

@jackmott

This comment has been minimized.

Copy link
Contributor

jackmott commented Jan 24, 2019

if anyone has an "IGP" or "nForce" nvidia card to run glxinfo on I'd like to see the output. the opengl database doesn't have any examples.

let renderer_lower = renderer.to_lowercase();
let strings_that_imply_integrated = [
" xpress", // space here is on purpose so we don't match express
"radeon hd 4200",

This comment has been minimized.

@kvark

kvark Jan 24, 2019

Member

any reason to not match just `"radeon hd "?

This comment has been minimized.

@jackmott

jackmott Jan 24, 2019

Contributor

yes, most radeon hd gpus are not integrated

@kvark

This comment has been minimized.

Copy link
Member

kvark commented Jan 24, 2019

oh, ok
let's proceed!
bors r+

bors bot added a commit that referenced this pull request Jan 24, 2019

Merge #2595
2595: add device_type inference to gl backend r=kvark a=jackmott

A first pass at inferring the device type.  If we see "intel" in the device string we can be confident that it is an integratedgpu.  If we see "nvidia", "ati", or "amd" it is either going to be discrete or have performance much better than the intel integrated gpu. Otherwise we set device_type to Other since we really have no idea.


Fixes #2593 
PR checklist:
- [ ] `make` succeeds (on *nix)
- [ ] `make reftests` succeeds
- [x] tested examples with the following backends:gl
- [x] `rustfmt` run on changed code


Co-authored-by: jack mott <jack.mott@gmail.com>
@bors

This comment has been minimized.

Copy link
Contributor

bors bot commented Jan 24, 2019

@bors bors bot merged commit 1d78597 into gfx-rs:master Jan 24, 2019

2 checks passed

bors Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment