1094803 - Update APZ documentation to provide more technical detail on the flow of input events

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

:roc and :mattwoodrow didn't have a clear idea of how the input events flow through the APZ code and how we deal with the different requirements. I wrote up some documentation to describe it. It's not perfect but it's better than nothing. This also allows me to follow our fancy new process and "update the documentation" as part of bug 1090398 where some of this changes.

CC'ing some other people who are working on APZ stuff and may be interested in this too.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Update docs

Comment 2

[Kartikaya Gupta (email:[e-mail])]

You can see the "pretty" version of the doc with these changes at the bug URL.

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

And the doxygen version linked from http://people.mozilla.org/~bgirard/doxygen/gfx/pages.html (once it's landed.)

Comment 4

[Botond Ballo [:botond]]

Comment on attachment 8518142 [details] [diff] [review]
Update docs

Review of attachment 8518142 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! r+ with comments addressed.

::: gfx/doc/AsyncPanZoom.md
@@ +149,5 @@
>  
>  Because the cross-thread messaging is asynchronous, reconciling the two types of scroll changes is a tricky problem.
> +Our design solves this using various flags and generation counters.
> +The general heuristic we have is that content-driven scroll position changes (e.g. scrollTo from JS) are never lost.
> +For instance, if the user is doing an async scroll with their finger and content does a scrollTo in the middle, then some of the async scroll would occur before the "jump" and the rest after the "jump".

Are newlines in this document meant to be significant? They don't seem to make a difference in the rendered document.

@@ +232,5 @@
> +  </li>
> + </ol>
> +</li>
> +<li value="10">
> +If events were queued as part of step 4(ii) they are now either processed (if the input block was not cancelled) or dropped (if it was cancelled).

It's not clear that the case where the timeout fires is also handled in step 10, even though steps 4(ii) and 9(iii) suggest that. I'd mention that more explicitly in step 10.

@@ +240,5 @@
> +
> +If the CSS touch-action property is enabled, the above steps are modified as follows:
> +<ul>
> +<li>
> + In step 4, the APZC also requires the allowed touch-action behaviours for the input event. Since this is not available, the events are always queued.

I'd phrase it as "This is not available yet, so the events are always queued."

@@ +257,5 @@
> +However, this is the current state of things on B2G and Metro.
> +Using the compositor thread as the input thread could work on some platforms, but may be inefficient on others.
> +For example, on Android (Fennec) we receive input events from the system on a dedicated UI thread.
> +We would have to redispatch the input events to the compositor thread if we wanted to the input thread to be the same as the compositor thread.
> +This introduces a potential for higher latency, particularly if the compositor does any blocking operations - (blocking SwapBuffers operations, for example).

You don't need both a dash and parentheses.

@@ +258,5 @@
> +Using the compositor thread as the input thread could work on some platforms, but may be inefficient on others.
> +For example, on Android (Fennec) we receive input events from the system on a dedicated UI thread.
> +We would have to redispatch the input events to the compositor thread if we wanted to the input thread to be the same as the compositor thread.
> +This introduces a potential for higher latency, particularly if the compositor does any blocking operations - (blocking SwapBuffers operations, for example).
> +As a result, the APZ code itself does not assume that the input thread will be the same as the Gecko main thread or the compositor thread, and is written to assume an independent input thread.

I'd remove "and is written to assume an independent input thread", because it might be interpreted as "assume the input thread is neither the Gecko thread nor the compositor thread", which is false.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

(In reply to Botond Ballo [:botond] from comment #4)
> Looks good! r+ with comments addressed.

Thanks, updated locally to address all comments.

> Are newlines in this document meant to be significant? They don't seem to
> make a difference in the rendered document.

No, I just like putting one sentence per line as I've found it works best for doing diffs.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/212a82853c10

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/212a82853c10