[WIP] Major refactor (breaking changes)

[puckey]

I spent some time looking into improving detect-gpu:

• it was returning incorrect results in several cases, which I was able to improve by changing the way getEntryVersionNumber functions
• implemented a bunch of tests
• slimmed down the footprint by splitting up the data and loading it conditionally (for this reason I had to retire the umd build. It does not support conditional loading)
• precomputing 'entry version numbers' and limiting models to check by above should also cause it to take up less cpu
• switched to gfxbench.com as data source, since it offers fps benchmark data, which solves Reporting a tier based on the rank of the gpu doesn't seem like the best idea #12
• moved blacklist check into the data files
• replaced device-ua dependency with plain js
• instead of returning a string, getGPUTier now returns a promise which resolves to an object with properties: tier, mobile (boolean), type (BENCHMARK | BLACKLISTED | WEBGL_UNSUPPORTED | FALLBACK), and model (the renderer string it matched – handy for tests).

Instead of looking at the changed files, it probably makes sense to look at my repo instead: https://github.com/puckey/detect-gpu

Open for feedback on this – I realise it is a lot in one go.

[TimvanScherpenzeel]

Excellent! Thank you for putting so much effort into this! I'll have a good look over the weekend and get back to you.

[puckey]

Thanks!

I am wondering if we shouldn't use the raw fps information instead of calculating percentiles, since the percentiles will change whenever a device comes on the scene with higher frame rates.

And I am considering switching to the following benchmark, which seems to allow for higher fpses on lower powered devices: https://gfxbench.com/result.jsp?benchmark=gfx50&test=756&text-filter=&order=median&ff-lmobile=true&ff-smobile=true&os-Android_gl=true&os-Android_vulkan=true&os-iOS_gl=true&os-iOS_metal=true&os-Linux_gl=true&os-OS_X_gl=true&os-OS_X_metal=true&os-Windows_dx=true&os-Windows_dx12=true&os-Windows_gl=true&os-Windows_vulkan=true&pu-dGPU=true&pu-iGPU=true&pu-GPU=true&arch-ARM=true&arch-unknown=true&arch-x86=true&base=device

[puckey]

@TimvanScherpenzeel (Maybe wait with adding commits to this branch – I am still changing things and don't want to deal with too many merges quite yet.)

[TimvanScherpenzeel]

I am wondering if we shouldn't use the raw fps information instead of calculating percentiles, since the percentiles will change whenever a device comes on the scene with higher frame rates.

And I am considering switching to the following benchmark, which seems to allow for higher fpses on lower powered devices: https://gfxbench.com/result.jsp?benchmark=gfx50&test=756&text-filter=&order=median&ff-lmobile=true&ff-smobile=true&os-Android_gl=true&os-Android_vulkan=true&os-iOS_gl=true&os-iOS_metal=true&os-Linux_gl=true&os-OS_X_gl=true&os-OS_X_metal=true&os-Windows_dx=true&os-Windows_dx12=true&os-Windows_gl=true&os-Windows_vulkan=true&pu-dGPU=true&pu-iGPU=true&pu-GPU=true&arch-ARM=true&arch-unknown=true&arch-x86=true&base=device

I think that would be a possible solution but we have to be careful that the resolutions the tests have been rendered on are consistent. In the test you mentioned the GTX 1660 TI reports higher framerates than the RTX 2080 TI very likely due to the resolution.

[puckey]

Good point – on desktop machines we could filter by '1920x1080' resolutions. But not sure what to do with mobile – we could perhaps even match up screen size to the device? hm

[TimvanScherpenzeel]

Good point – on desktop machines we could filter by '1920x1080' resolutions. But not sure what to do with mobile – we could perhaps even match up screen size to the device? hm

I think on desktop devices it makes sense to filter or come up with some formula to describe fps per pixel (this could raise issues with pixel density). I wouldn't worry about screen size difference on mobile as I presume they tested on the physical phone and not an external screen so the fps in that sense is "real".

[TimvanScherpenzeel]

GTX 1660 TI -> 723.1 / (1195 x 768) = 0,000787896617852
RTX 2080 TI -> 656.8 / (1920 x 1080) = 0,00031674382716

Sorting by the lowest would then yield the highest relative performance. We might run into some floating point error issues at some point but generally this would work I think

[puckey]

Good point – on desktop machines we could filter by '1920x1080' resolutions. But not sure what to do with mobile – we could perhaps even match up screen size to the device? hm

I think on desktop devices it makes sense to filter or come up with some formula to describe fps per pixel (this could raise issues with pixel density). I wouldn't worry about screen size difference on mobile as I presume they tested on the physical phone and not an external screen so the fps in that sense is "real".

A GPU could be used in phones of different screen resolutions. For example the A12 bionic is used in these devices:

• Apple iPad Air 2019
• Apple iPad Mini 5
• Apple iPhone XR
• Apple iPhone XS
• Apple iPhone Xs Max

[puckey]

How about the data structure I just pushed: 6056a32#diff-2e61470ff9838f404e727964eb374441

the fourth element in the array is an array of [screenWidth, screenHeight, fps, deviceName] (device name is not included in desktop data, because it is the same as the GPU name)

We could use this to both match the performance impact of large displays on desktop as well as better match specific mobile devices.

[TimvanScherpenzeel]

Great! I think that would work well 👍

What would the mobileTiers and desktopTiers now signify?

[TimvanScherpenzeel]

(and if there is not much of a speed difference, I guess we should go for the smallest footprint gzip size wise)

I'll create a new ticket once this is merged, leven should do the trick for now.

[puckey]

I think functionality wise things are nearly done with the refactor. I spent some time today looking into the deobfuscation of the apple gpu's. I refactored deobfuscateAppleGpu to return multiple possible renderers and refactored getGPUTier to be able to work with an array of renderers internally.

I did some brief tests on browserstack and it looks like the library is now able to pinpoint most individual apple devices thanks to the combination of GPU detection, screen size comparisons and filtering based on whether the device is an ipad or not.

On the android side of things we are also able to pinpoint devices more accurately, but it seems the benchmark data is a little out of date on that side right now. Perhaps there are more benchmark results out there somewhere we could use.

The amd version is now 2.98kb gzipped.

[puckey]

I also clarified the api a bit by moving overridable parameters together:

type GetGPUTierParameters = {
  glContext?: WebGLRenderingContext | WebGL2RenderingContext;
  failIfMajorPerformanceCaveat?: boolean;
  mobileTiers?: number[];
  desktopTiers?: number[];
  benchmarksUrl?: string;
  override?: {
    renderer?: string;
    isIPad?: boolean;
    isMobile?: boolean;
    screen?: { width: number; height: number };
    loadBenchmarks?: (file: string) => Promise<ModelEntry[] | undefined>;
  };
}

[TimvanScherpenzeel]

Great! I noticed there are quite of few linting errors, would you like me to directly fix it on your branch?

[TimvanScherpenzeel]

I also clarified the api a bit by moving overridable parameters together:

type GetGPUTierParameters = {
  glContext?: WebGLRenderingContext | WebGL2RenderingContext;
  failIfMajorPerformanceCaveat?: boolean;
  mobileTiers?: number[];
  desktopTiers?: number[];
  benchmarksUrl?: string;
  override?: {
    renderer?: string;
    isIPad?: boolean;
    isMobile?: boolean;
    screen?: { width: number; height: number };
    loadBenchmarks?: (file: string) => Promise<ModelEntry[] | undefined>;
  };
}

Looks good, I like the idea.

[TimvanScherpenzeel]

Great! I noticed there are quite of few linting errors, would you like me to directly fix it on your branch?

I have about 15 commits on stand-by fixing all lint errors and improving the readability. Please let me know if I can push this to your branch.

Regarding the dist/ directory, I think we should remove it from git, this will make future PR's also a lot easier to handle.

Notes for me:

• Update README / documentation
• Document individual functions
• Make sure we correctly expose TypeScript types
• Sort out missing types in unfetch
• Resolve any and tslint ignores
• Replace TSLint with ESLint (with TypeScript support)
• Create CHANGELOG informing users with breaking changes
• Do some more extensive testing
• Inform previous contributors, see if they have feedback
• Inform the developer community on Twitter
• Fix live-demo that lives on gh-pages
• Clean up, prepare for a 2.0 alpha release

Overall I am really happy with all the new additions (especially the more accurate reporting and that the work of #26 is included in here). Thank you for all the effort so far 👍 !

[puckey]

Great! I noticed there are quite of few linting errors, would you like me to directly fix it on your branch?

I have about 13 large-ish commits on stand-by fixing all lint errors and improving the readability. Please let me know if I can push this to your branch.

Yes, go ahead. How about we merge the pull request into the development branch and you take it from there?

After checking with TSLint I was wondering about some of the rules:

• expected arrow-call-signature to have a typedef: in my opinion adding return types to arrow functions is unnecessary unless typescript is unable to infer. It also leads to more work during development, since you keep having to adjust every single arrow function type whenever a large change is made up the chain. I would suggest we lean more on typescript's power to infer types (it will warn when any type becomes untyped any anyway, since we are in strict mode)
• no-shadowed-variable: in my experience this can lead to less legible code when followed strictly..
• prefer-const: this rule leads to situations where it doesn't want you destructuring into a let when you are only reassigning one of the destructured values - which seems a bit pedantic to me..

Another option would be to skip eslint altogether and opt for a combination of prettier & typescript to cover most code quality cases...

Regarding the dist/ directory, I think we should remove it from git, this will make future PR's also a lot easier to handle.

Good point!

[TimvanScherpenzeel]

Great! I noticed there are quite of few linting errors, would you like me to directly fix it on your branch?

I have about 13 large-ish commits on stand-by fixing all lint errors and improving the readability. Please let me know if I can push this to your branch.

Yes, go ahead. How about we merge the pull request into the development branch and you take it from there?

After checking with TSLint I was wondering about some of the rules:

• expected arrow-call-signature to have a typedef: in my opinion adding return types to arrow functions is unnecessary unless typescript is unable to infer. It also leads to more work during development, since you keep having to adjust every single arrow function type whenever a large change is made up the chain. I would suggest we lean more on typescript's power to infer types (it will warn when any type becomes untyped any anyway, since we are in strict mode)
• no-shadowed-variable: in my experience this can lead to less legible code when followed strictly..
• prefer-const: this rule leads to situations where it doesn't want you destructuring into a let when you are only reassigning one of the destructured values - which seems a bit pedantic to me..

Another option would be to skip eslint altogether and opt for a combination of prettier & typescript to cover most code quality cases...

Regarding the dist/ directory, I think we should remove it from git, this will make future PR's also a lot easier to handle.

Good point!

Sounds good! I'll merge it into development.