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

Clarify renderWidth/Height: recommended size or 1:1? #125

Closed
toji opened this issue Oct 17, 2016 · 5 comments
Closed

Clarify renderWidth/Height: recommended size or 1:1? #125

toji opened this issue Oct 17, 2016 · 5 comments
Milestone

Comments

@toji
Copy link
Member

toji commented Oct 17, 2016

On some devices, especially mobile, there are two potentially width and height values for the render targets we could report. One is the size required for the pixels at the center of the users vision to be 1:1 with the physical screen pixels post-distortion. The other is the size of the buffer that the platform feels provides the best balance of quality/performance. For example: on GearVR the reported value is 1024x1024, even though the screen could benefit from higher. It's explained in their documentation that this value will hit a sweet spot for most apps, though something like a photo viewer may want to use a larger target.

Which of these values should WebVR report? Or should we report both?

@brianchirls
Copy link

I've been pretty happy so far with modifying the resolution on the fly to keep the frame rate up, so I'd recommend sticking with the greater of the two values. You can always step it down. Also, I correct me if I'm wrong here but the fill rate could be widely variable depending on the complexity of the fragment shader(s) and the amount of overdraw. So I'm not convinced the recommended value is useful. Even if you do use a fixed resolution, you're still gonna have to benchmark on a few devices.

@toji
Copy link
Member Author

toji commented Oct 18, 2016

Glad to hear that dynamic resolution is working well for you! My concern is that a lot of naive applications will just allocate a buffer at the size that we report and be done with it, even if it's not what they really want from a performance perspective. Then again maybe we should just build dynamic resolution adaptation into Three.js and that'll solve it for 90% of devs. ;)

Regardless of what we chose, the spec text should be more explicit about what the value represents.

@mkeblx
Copy link
Contributor

mkeblx commented Nov 3, 2016

Ideally every app would likely do some level (basic or advanced) of dynamic scaling and that is true also of non-VR WebGL apps where currently not usually employed. I don't see a way around besides app/framework handling in some fashion.

I don't know if we need two different values sets though.. "good balance of quality and performance for most applications" is going to be pretty arbitrary and likely on the conservative side well below 1:1 i.e. Oculus Mobile SDK recommendation of 1024x1024 even for native (with specific apps like photo app using larger (or using OVR Layers)) for all supported phones (all within a certain ~small factor of perf, same screen resolution [2560x1440], which won't be true of what WebVR content running on) but I don't know how they will change with increasing resolutions, more GPU power, ..

Native is going to face the same problem, I wonder their thoughts on. Do you know any thoughts from Google VR SDK native side? From cursory (could be wrong) looking it looks like someone moving from a Pixel (1080p) to a next-gen 4K @ 120Hz screen Daydream phone going to suddenly need 4x the perf as sizes based on screen size? (Native) App developer would have to handle dynamic in this case if didn't want 1/8 framerate. Developers responsibility.

Maybe just go with something like 1:1 and call recommendedRenderWidth/Height?

+1 on doing lots of things the best way in three.js, best practices, etc..

@toji
Copy link
Member Author

toji commented Dec 15, 2016

Revisiting this, since I've now seen it be a sticking point for several different WebVR sites and browser implementations. Some points worth noting:

  • In some cases (whether as a bug or limit of the underlying API) the recommended resolution values aren't available until presentation starts. In Chrome for Android this meant that they were dummy values until the vrdisplaypresentchange event was fired.
  • Some sites tried to query the resolution prior to presenting, and would thus end up with bad values.
  • Even when they were querying it at the right time sometimes they would scale the value by a multiplier (like 1.2) and end up with a higher resolution than intended, which had a severe impact on performance.
  • Most sites want a resolution that is good for performance (which ends up being surprisingly low in most mobile cases), but I've also heard from developers that have specific needs for 1:1 pixel ratios (they're presenting text and care more about readability than performance.) So ideally we want a way to get both values still.
  • I've also heard back from the W3C TAG review that in general it's preferred to not have properties like this be attributes but instead surface them when they're most relevant somehow.

Given all of the above, I've got a new proposal for how to handle communicating HMD resolution. For VRLayers, add a resolution scale value, like so:

dictionary VRLayerInit {
  // ... other existing properties
  float? resolutionScale = null;
};

The default of null (or perhaps 0.0 or something similar) means "Give me the recommended value for the best balance of performance and quality", and the system is free to decide what exactly that means. Anything else is a scalar applied to the full 1:1 pixel ratio resolution. So 1.0 give you a canvas size that matches the output resolution 1:1 at the center of the users view, 0.5 is half that, 1.2 is 120% of 1:1 (for supersampling, though maybe we want to allow clamping?)

The resolution itself could be surfaced in one of two ways: Either via the requestPresent promise resolve or simply by setting the canvas size to the requested resolution directly.

Passing the resolution to the promise resolve is pretty simple from an API point of view, but a bit of a pain for the developer. Consider that we'll want to eventually support N layers, each of which may want it's own scalar, so we can't just pass back a single width/height. Realistically the API would look something like this:

interface VRLayer {
  float resolutionScale; // If set to default value in init dictionary this should the reflect actual value.
  // Resolved dimensions based on requested resolutionScale.
  unsigned width;
  unsigned height;
};

interface VRDisplay {
  Promise<sequence<VRLayer>> requestPresent(sequence<VRLayerInit> layers);
};

Used like so:

vrDisplay.requestPresent([ { source: canvas, resolutionScale: 1.0 } ])
  .then(function(layers) {
    for (let layer of layers) {
      layer.source.width = layer.width;
      layer.source.height = layer.height;
    }
  });

Which, admittedly, feels a little silly. But it's scalable!

In that light, setting the canvas size directly is appealing superficially because it magically makes sure that the right thing happens in almost all cases. It's also a bit distressing because it "magically" makes sure that the right thing happens, and nothing is more frustrating to a developer than when the magic fails. It also comes with a bunch of behavioral questions: Do we restore the old size after presentation ends? Should we allow manual resizing during presentation? If we extend layers to include non-canvas types what's the expected behavior? I'm also not sure if there's a precedent for an API like this so directly manipulating element dimensions, so maybe it would get some pushback by virtue of simply being weird.

In either case, though, I feel like this fulfills several goals by not surfacing the value till it's needed, (fingerprinting entropy--, BTW!), giving the user control over quality while making the default case sensible, and ensuring that any resizing that does happen MUST happen at the right time. Thoughts?

toji added a commit that referenced this issue Jan 7, 2017
Attempts to address #142, #107, #166, #156, and #125. Feedback welcome!
@toji toji added this to the 1.2 milestone Feb 7, 2017
@toji
Copy link
Member Author

toji commented Mar 8, 2017

Closing since this is planned to be addressed in 2.0. If anyone has issues with the 2.0 proposal, please file a new issue. For details see: https://github.com/w3c/webvr/blob/master/explainer.md#high-quality-rendering

@toji toji closed this as completed Mar 8, 2017
@cwilso cwilso modified the milestones: Spec-Complete for 1.0, 1.0 Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants