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

Add lowLatency flag to CanvasRenderingContext2DSettings. #4234

Closed
wants to merge 4 commits into from

Conversation

@kenrussell
Copy link
Contributor

commented Dec 11, 2018

Developers of stylus-based drawing applications (e.g. Google Keep)
have found this hint to be critical in order to be competitive with
native applications. It has been prototyped in Chromium and yields
significant latency improvements there.

Partial fix for #4087.


/canvas.html ( diff )
/references.html ( diff )

@kenrussell

This comment has been minimized.

Copy link
Contributor Author

commented Dec 11, 2018

@annevk @domenic please review.

@yellowdoge @fserb @wffurr FYI.

kenrussell added a commit to kenrussell/WebGL that referenced this pull request Dec 11, 2018
Add lowLatency flag to WebGLContextAttributes.
Developers of stylus-based drawing applications (e.g. Google Keep)
have found this hint to be critical in order to be competitive with
native applications. It has been prototyped in Chromium and yields
significant latency improvements there.

This is the WebGL analogue of whatwg/html#4234
and partially addresses whatwg/html#4087 .
@annevk

This comment has been minimized.

Copy link
Member

commented Dec 12, 2018

@annevk
Copy link
Member

left a comment

I don't think this works as "front buffer" is not an established term. Searching for it suggest origins in OpenGL, but WebGL doesn't mention it. I think it would be good to be a little clearer or provide a reference. The same goes for "tearing", but that's less problematic as it's not a normative statement.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@domenic
Copy link
Member

left a comment

This is intentionally only for 2D, right? Not OffscreenCanvasRenderingContext?

I think @annevk's feedback about front buffering could be addressed in two ways:

  1. Adding a normative reference to something that explains it to the end of the paragraph. This could be something pretty general, e.g. a canonical computer graphics book; cf. https://html.spec.whatwg.org/#refsBEZIER

  2. Moving the specific mention of front buffer into something non-normative, e.g. <p class="note">One implementation strategy for reducing latency is to render from the front buffer.</p>

I guess you could do both, in fact.

Overall seems pretty solid to me, just some minor tweaks.

source Outdated Show resolved Hide resolved
@kenrussell

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2018

Added a citation for front-buffering as well.

@kenrussell

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2018

Also, yes, the intent here is to support this for the 2D and WebGL rendering contexts. It doesn't matter whether they're fetched against an HTMLCanvasElement or OffscreenCanvas. The PR adding this to WebGL is KhronosGroup/WebGL#2753 .

@domenic
Copy link
Member

left a comment

Sorry, I thought that there was an OffscreenRenderingContext, but I was confused :).

This LGTM with minor markup/style issues. @annevk should confirm, but I am hopeful. Thanks for doing this!!

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
kenrussell added a commit to kenrussell/WebGL that referenced this pull request Dec 18, 2018
Add lowLatency flag to WebGLContextAttributes.
Developers of stylus-based drawing applications (e.g. Google Keep)
have found this hint to be critical in order to be competitive with
native applications. It has been prototyped in Chromium and yields
significant latency improvements there.

This is the WebGL analogue of whatwg/html#4234
and partially addresses whatwg/html#4087 .
@domenic

This comment has been minimized.

Copy link
Member

commented Dec 18, 2018

I realized this doesn't have multi implementer interest yet, so I asked in #4087 (comment).

Is it possible to write web platform tests for this? Not really, I imagine. Maybe if we also added #2563 to the spec, then surface-level tests could be added to see the parameter reflected back. But until then I don't think we should block this PR landing on tests. Thoughts?

@kenrussell

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

We should certainly land #2563 . It's important that Canvas2D have the same introspection capabilities as WebGL. Would gladly write a surface-level test for it then.

It's not really possible to write even a wpt test that guarantees that the browser went into low-latency mode. For the record, this flag has been implemented behind a flag in Chromium (by @yellowdoge, @fserb and @junov) and is delivering good latency improvements on real world drawing apps.

kenrussell added a commit to kenrussell/WebGL that referenced this pull request Dec 18, 2018
Add lowLatency flag to WebGLContextAttributes.
Developers of stylus-based drawing applications (e.g. Google Keep)
have found this hint to be critical in order to be competitive with
native applications. It has been prototyped in Chromium and yields
significant latency improvements there.

This is the WebGL analogue of whatwg/html#4234
and partially addresses whatwg/html#4087 .
@annevk

This comment has been minimized.

Copy link
Member

commented Jan 3, 2019

I don't quite understand why "should" was rejected here. The HTML Standard has plenty of should requirements and it seems appropriate here. What did the linter say? Otherwise this seems fine to me, thanks for digging up a reference.

source Outdated Show resolved Hide resolved
@domenic

This comment has been minimized.

Copy link
Member

commented Jan 3, 2019

This PR contains largely duplicate text in the nonnormative domintro and in the normative portion, so presumably the linter rejected "should" when it was used in the nonnormative portion.

@jrmuizel

This comment has been minimized.

Copy link

commented Jan 3, 2019

@kenrussell is there some documentation someplace on the Chromium implementation that describes what was done to get the gains that were seen?

@yellowdoge

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2019

@jrmuizel see the explainer used in the intent-to-experiment at https://tinyurl.com/lowlatency-canvas-on-chromeos, IIRC there are a few linked docs under it with more information.

@jrmuizel

This comment has been minimized.

Copy link

commented Jan 3, 2019

@yellowdoge thanks. Are there other implementations planed beyond ChromeOS?

@yellowdoge

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2019

@yellowdoge thanks. Are there other implementations planed beyond ChromeOS?

Not under the scope of the given experiment.

@kenrussell kenrussell force-pushed the kenrussell:add-canvas-2d-lowlatency branch from 21f27cc to 1311a13 Jan 11, 2019

@kenrussell

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2019

Rebased on top of #4253 , adding the lowLatency flag to the result of getContextAttributes(). Please re-review. Thanks.

@annevk
Copy link
Member

left a comment

Thanks, looks good generally, but there's some nits left. This also still needs implementation bugs filed.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@yellowdoge

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2019

@annevk, @kenrussell

This also still needs implementation bugs filed.

kenrussell added 3 commits Dec 11, 2018
Add lowLatency flag to CanvasRenderingContext2DSettings.
Developers of stylus-based drawing applications (e.g. Google Keep)
have found this hint to be critical in order to be competitive with
native applications. It has been prototyped in Chromium and yields
significant latency improvements there.

Partial fix for #4087.

@kenrussell kenrussell force-pushed the kenrussell:add-canvas-2d-lowlatency branch from 1311a13 to feeb223 Jan 21, 2019

nit
@annevk

This comment has been minimized.

Copy link
Member

commented Jan 22, 2019

@jrmuizel is Mozilla okay with this?

@grorg Apple?

@yellowdoge

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2019

The lowLatency experiment in Chrome is reaching its end, and in order to ship it we'd need a spec update, so: friendly ping @annevk, @jrmuizel, @grorg.

@annevk

This comment has been minimized.

Copy link
Member

commented Jan 24, 2019

Does that also mean it would no longer be restricted to ChromeOS? That wasn't entirely clear to me from the earlier conversation.

@yellowdoge

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

#4234 (comment) : the current experiment, which should lead to shipping, is only implemented for CrOs, all other platforms will stay the same as of M73. However, in any case we still need a spec update (this PR) to ship it; since the current wording is that lowLatency is a hint, that may or may not be honoured by the UA, I see little risk to deliver what we have and investigate other platforms' front buffering implementations afterwards.

@travisleithead

This comment has been minimized.

Copy link

commented Jan 24, 2019

I'm a little concerned that the name of the hint implies only positive benefits without a sense of the potential consequences for turning it on (e.g., potential tearing). After all, who wouldn't want "low latency" in their application. Have alternative names already been bikeshedded and if so, where is the discussion?

@yellowdoge

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2019

@travisleithead do you have any concrete alternative? frontBuffer rendering was deemed not general enough to be understood (can find the concrete link, but it was raised during this or the sibling WebGL PR review). OTOH not everyone wants lowLatency, e.g. streaming/video likes to buffer a few (or a lot of) video frames to decrease playback jitter and/or avoid interruptions.

@travisleithead

This comment has been minimized.

Copy link

commented Jan 29, 2019

I could spitball a few names, I suppose. After talking over the behavior with some folks internally, it seems the major behavioral difference is about what process comes along to render-out the canvas once its been dirtied. Ordinarily, this is the render loop (when all the browser's DOM, Layout, Rendering changes get applied at once and pushed out. With this new canvas flag, another entity may swoop in and render-out the canvas independently of the render loop (and at higher frequency perhaps) or it may be "push" driven--any time any canvas changes are committed they get rendered out (I'm not sure which it is, and implemenations may vary I suppose). So, some alternative names to consider: preferIndependentRendering, preferFreeRendering, unalignedRendering, or preferDirectRendering.

@jdashg

This comment has been minimized.

Copy link

commented Jan 31, 2019

unbuffered:true has a vague latency-vs-throughput implication in general, so maybe that's an option for the name?

Alternatively, it seems that the core benefit here comes from desynchronizing from DOM updates. Maybe synchronized:false is better?

Is there not a more specific term for this desynchronization? I've been calling this "DOM (update) atomicity".

Also, is this compatible with antialias:true or preserveDrawingBuffer:false? @kenrussell I think it's fine if it isn't, but I don't think it can be right now.

@kenrussell

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2019

lowLatency:true is compatible with both WebGL options antialias:true and preserveDrawingBuffer:false. (Chrome supports both.) With antialias:true, the single-sampled buffer is the unsynchronized output buffer, and the user's multisampled rendering results are blitted into it at the end of the frame. With preserveDrawingBuffer:false, an intermediate buffer to hold the user's current rendering is again introduced, and a blit's done at the end of the frame to the unsynchronized output buffer.

It's a fair point that in these two scenarios there's not much more advantage than desynchronizing from the surrounding DOM updates, but antialias:false and preserveDrawingBuffer:true do provide the option of true front-buffer rendering, and we've been experimenting with that in order to try to reach the same latencies as hand-tuned native applications with the web platform.

@othermaciej

This comment has been minimized.

Copy link
Collaborator

commented Feb 4, 2019

From the PR, I can't tell what this does. Maybe because I don't know what a "front buffer". Also not clear to me why optimizing for latency would have the stated side effects. And finally, it's not clear to me why this is needed. Canvas can't paint faster than the display's maximum refresh rate, and keeping up with a stylus shouldn't require going faster than that. Maybe there is hardware where this is not true and it is possible to paint faster than the refresh rate, but that's not something I (a graphics non-expert) am familiar with. Or maybe this is solely a worry that other acitivity could cause dropped frames but canvas frames shouldn't be dropped. I will try to get WebKit folks who understand graphics better (@grorg @smfr @graouts) to weigh in.

I agree with others that the name is unclear (since it implies only benefits) and that the spec isn't precise enough for interoperable implementation (though maybe just because I am missing important domain expertise).

@yellowdoge

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2019

Please follow up the conversation after #4360 (comment), thanks!

@kenrussell

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

Closing in favor of #4360 , as @yellowdoge is the one who has implemented this support in Chrome and is best suited to discuss changing the name.

@kenrussell kenrussell closed this Feb 12, 2019

annevk added a commit that referenced this pull request Mar 20, 2019
Add an optional desynchronized mode to 2D canvas
Developers of stylus-based drawing applications (e.g., Google Keep) have found this hint to be critical in order to be competitive with native applications. It has been prototyped in Chromium and yields significant latency improvements there.

Tests: will be added as part of https://bugs.chromium.org/p/chromium/issues/detail?id=788439.

Supersedes #4234. Partial fix for #4087.

Co-authored-by: Miguel Casas-Sanchez <mcasas@chromium.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
8 participants
You can’t perform that action at this time.