search

borismus / webvr-boilerplate

A starting point for web-based VR experiences that work on all VR headsets.
Apache License 2.0
1.8k stars 451 forks source link

Dpdb #103

Closed btco closed 8 years ago

btco commented 8 years ago

Adds the DPDB file which contains DPI and bevel width information for several popular devices. This is now used instead of the hard-coded device parameters in device-info.js.

borismus commented 8 years ago

Looks great overall, but I left some minor nits here and there.

brianchirls commented 8 years ago

This is generally great - it's good to support as many devices as possible. Thanks for submitting. But I have a few issues in addition to the ones @borismus commented on...

pindiespace commented 8 years ago

How do these values compare to those on http://www.devicespecifications.com/ (it lists screen widths and heights independently of diagonals)

On Fri, Jan 15, 2016 at 7:46 PM, Brian Chirls notifications@github.com wrote:

This is generally great - it's good to support as many devices as possible. Thanks for submitting. But I have a few issues in addition to the ones @borismus https://github.com/borismus commented on...

  • The big JSON blob should be unminified in the repo. That way it will be readable and any future commits will make sense in diffs.
  • I see a lot of numerical values in there that are included as strings. Those quotes should be removed. It will make the file smaller, and there won't be any chance of non-numerical values accidentally slipping in later because they simply won't validate.
  • Similarly, dpi and res should be broken down into numbers rather than have the javascript parse them out.You could list them as two-element arrays. If you wanna be really compact, optionally allow a single number that is then interpreted as the value for both x and y.
  • I don't like that the file is loaded by XHR. If this project were using webpack, I'd recommend using a json loader with require, but as it is I'd rather see it included as code. That way whoever is using the code can decide on their own whether to concatenate or load asynchronously. And it can be minified better as code, eliminating unnecessary quotes. It will also allow for comments in the code in case there's something weird about one of the entries that needs to be explained.
  • (At the very least, a JSON file should have a .json extension, not .txt.)

— Reply to this email directly or view it on GitHub https://github.com/borismus/webvr-boilerplate/pull/103#issuecomment-172152024 .

Dr. Pete Markiewicz Email: pindiespace@gmail.com Sustainable virtual design blog: http://sustainablevirtualdesign.wordpress.com Sustainability template: http://greenboilerplate.com Teaching site: http://plyojump.com http://www.plyojump.com/ Buy my book! - Millennials and the Popular Culture Lifecourse: http://www.lifecourse.com/store/catalog/lca/mpc.html

brianchirls commented 8 years ago

This all looks interesting. I will dig in tomorrow and see if this has any effect on #102 and #105. It's definitely good that there's a default iOS device, which I imagine will at least prevent some breaking errors.

@btco, can you share a link to documentation on DPDB? I can't seem to find any info on who compiles it, how it works or how often it's updated? Thanks.

btco commented 8 years ago

I'll add the documentation to the README soon! On Jan 21, 2016 7:57 PM, "Brian Chirls" notifications@github.com wrote:

This all looks interesting. I will dig in tomorrow and see if this has any effect on #102 https://github.com/borismus/webvr-boilerplate/issues/102 and #105 https://github.com/borismus/webvr-boilerplate/issues/105. It's definitely good that there's a default iOS device, which I imagine will at least prevent some breaking errors.

@btco https://github.com/btco, can you share a link to documentation on DPDB? I can't seem to find any info on who compiles it, how it works or how often it's updated? Thanks.

— Reply to this email directly or view it on GitHub https://github.com/borismus/webvr-boilerplate/pull/103#issuecomment-173799643 .

pindiespace commented 8 years ago

Size matrix on this app page: http://www.paintcodeapp.com/news/ultimate-guide-to-iphone-resolutions