Bug 103647 - [chromium] Mark last input event for current vsync interval
Summary: [chromium] Mark last input event for current vsync interval
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sami Kyöstilä
URL:
Keywords:
Depends on:
Blocks: 103648
  Show dependency tree / graph
 
Reported: 2012-11-29 09:34 PST by Sami Kyöstilä
Modified: 2012-12-03 04:40 PST (History)
7 users (show)

See Also:

Attachments
Patch (2.57 KB, patch)
2012-11-29 09:37 PST, Sami Kyöstilä 		no flags Details | Formatted Diff | Diff
Patch (1.70 KB, patch)
2012-11-30 06:56 PST, Sami Kyöstilä 		no flags Details | Formatted Diff | Diff
Patch (2.04 KB, patch)
2012-12-03 03:41 PST, Sami Kyöstilä 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sami Kyöstilä 2012-11-29 09:34:04 PST 
[chromium] Mark last input event for current vsync interval
Comment 1 Sami Kyöstilä 2012-11-29 09:37:12 PST 
Created attachment 176749 [details]
Patch
Comment 2 WebKit Review Bot 2012-11-29 09:40:45 PST 
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 James Robinson 2012-11-29 18:19:05 PST 
Comment on attachment 176749 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=176749&action=review

> Source/WebKit/chromium/public/WebInputEvent.h:169
> +    bool endOfInput; // Last input event for the current vsync interval.

I think this name needs to be a bit more descriptive.  The comment is good

> Source/WebKit/chromium/src/WebInputEvent.cpp:47
> +    int inputData[6];

that's sad - can we fit this in somewhere else? modifiers seems to have plenty of bits available
Comment 4 Sami Kyöstilä 2012-11-30 06:56:20 PST 
Created attachment 176961 [details]
Patch

How's this?
Comment 5 James Robinson 2012-11-30 11:45:12 PST 
Comment on attachment 176961 [details]
Patch

I think it's pretty good!  Darin/Adam - does this make enough sense for folks who aren't living in compositor land all day, or do you think we should segregate this out some more?
Comment 6 Adam Barth 2012-11-30 13:51:17 PST 
Comment on attachment 176961 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=176961&action=review

> Source/WebKit/chromium/public/WebInputEvent.h:162
> +        // Last input event to be sent for the current vsync interval.
> +        IsLastInputEventForCurrentVSync = 1 << 13,

Seems fine to me.  Will this bit be set accurately on every OS?  If not, it might be worth explaining in the comment that this is OS(ANDROID) specific.
Comment 7 James Robinson 2012-11-30 15:42:56 PST 
I think it'll be false by default, which is a fine value to set since that means the sender of the input events can't promise that this is the last input of a vsync.  It's only set when the embedder has knowledge.
Comment 8 James Robinson 2012-11-30 15:43:17 PST 
Comment on attachment 176961 [details]
Patch

R=me but documenting what it means for the flag to be false would help.
Comment 9 Sami Kyöstilä 2012-12-03 03:41:16 PST 
Created attachment 177224 [details]
Patch

Patch for landing.
Comment 10 WebKit Review Bot 2012-12-03 04:40:39 PST 
Comment on attachment 177224 [details]
Patch

Clearing flags on attachment: 177224

Committed r136383: <http://trac.webkit.org/changeset/136383>
Comment 11 WebKit Review Bot 2012-12-03 04:40:43 PST 
All reviewed patches have been landed.  Closing bug.