Have a front end to automatically graph touch event input move velocity to the scrollable layer's transform to measure frame uniformity.

Cool~ I wanna know if it could make a loop test like this one I hacked in InputReaderThread. https://drive.google.com/file/d/0B4nXGzdjFCO7Sl9NNkxXX1A4T2c/edit?usp=sharing The test cases should at least cover(hit the scenarios) like horizontal move, vertical move and APZC.

http://htmlpreview.github.io/?https://github.com/JinhoHwang/FxOSFrameUniformity/blob/master/FrameUniformity.html This is a tool which draws graphs related with frame uniformity, touch events and layer movement. How to use this Step1 : apply patches attached on this Step2 : build your gecko and download it to your device Step3 : add a preference to user preference file ex) echo "pref('layers.scroll-graph',true);" >> /system/b2g/defaults/pref/user.js Step4 : do some dragging or scrolling on device if you use Orangutan, it allows us to make constant movement Step5 : collect scrollgraph log data ex) adb log cat | grep ScrollGraph step5 : open the html file in your browser and put the log data into the textarea and click the "Reload data" button.

Cool! Let's get this checked in, the frame uniformatity.html too if it's standalone.

(In reply to Benoit Girard (:BenWa) from comment #3) > Cool! Let's get this checked in, the frame uniformatity.html too if it's > standalone. Yes, It's standalone. I reuse the code of bgirard's Rendertrace(which is greatly useful to understand behavior of APZ). but I'm not a JS guy, any help or feedback welcome. Since this is using ScrollGraph log data, this is dependent on Bug 982534.

I'll get this hosted on people.mozilla.org. Thanks for the tool! Where should we check in frame uniformaity.html? Mozilla-central?

Just a note to myself to host this on people.moz.

Comment on attachment 8403817 [details] [diff] [review] Patches for Frame Uniformity tool Review of attachment 8403817 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gonk/nsAppShell.cpp @@ +713,4 @@ > break; > case AMOTION_EVENT_ACTION_POINTER_DOWN: > case AMOTION_EVENT_ACTION_POINTER_UP: > + return; What's this change? Should it be inside #ifdef SCROLL_GRAPH, or is it a fix for some other issue unrelated to this?

(In reply to Milan Sreckovic [:milan] from comment #9) > Comment on attachment 8403817 [details] [diff] [review] > Patches for Frame Uniformity tool > > Review of attachment 8403817 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: widget/gonk/nsAppShell.cpp > @@ +713,4 @@ > > break; > > case AMOTION_EVENT_ACTION_POINTER_DOWN: > > case AMOTION_EVENT_ACTION_POINTER_UP: > > + return; > > What's this change? Should it be inside #ifdef SCROLL_GRAPH, or is it a fix > for some other issue unrelated to this? Thanks you for the review. You're right. I made a mistake. I changed the code because of a Orangutan bug. So, I should open a bug for it and remove it from here.

(In reply to Mason Chang [:mchang] from comment #7) > I'll get this hosted on people.mozilla.org. Thanks for the tool! > > Where should we check in frame uniformaity.html? Mozilla-central? We could check it in to gfx/layers but it hotlinks several JS libraries so it's not good style to have files in mozilla-central that hotlink resources. Maybe for now hosting it on people.mozilla.org is the right thing to do.

Hosted on people.mozilla.org - http://people.mozilla.org/~mchang/FrameUniformity.html

Jeff, any news on this patch? Is it possible to please get this reviewed and landed? Thanks!

Sorry for the delay in updates. Patch for review. Mason could you please update http://people.mozilla.org/~mchang/FrameUniformity.html this html file?

I've been rolling some of this into the profiler to make this easier to use. But I don't track input events: http://benoitgirard.wordpress.com/2014/05/05/profiler-frame-performance-and-debugging/ (The layer movement uniformity was added after this blog posted)

(In reply to Benoit Girard (:BenWa) from comment #15) > I've been rolling some of this into the profiler to make this easier to use. > But I don't track input events: Wow, great ~ :) it's nice to have frame uniformity tool inside of profiler. One question about the tool, the graph on your blog looks like representing FPS. If the dots are near 16ms, that means 60FPS. Is there any extra information for movements of layers? If it provides that informations, it will be much useful thought.. Anyway, I'll just keep this patch here for the project Slik in progress.

Comment on attachment 8420791 [details] [diff] [review] Uniformity.patch Review of attachment 8420791 [details] [diff] [review]: ----------------------------------------------------------------- Mostly looks good. Please fix the nits and ask for review again. ::: gfx/layers/composite/ContainerLayerComposite.cpp @@ +247,5 @@ > + aLayer->GetEffectiveVisibleRegion().GetBounds().height < 300) { > + return; > + } > + > + FrameMetrics fm = aLayer->AsContainerLayer()->GetFrameMetrics(); nit: Please rename to frameMetrics. Short variable names are difficult to read. ::: widget/gonk/nsAppShell.cpp @@ +67,5 @@ > #ifdef MOZ_NUWA_PROCESS > #include "ipc/Nuwa.h" > #endif > > +#ifdef UNIFORMITY_INFO I generally don't think we need to #ifdef this. Can we remove all the #ifdef UNIFORMITY_INFO. @@ +232,5 @@ > touch.coords.getAxisValue(AMOTION_EVENT_AXIS_PRESSURE)) > ); > } > > +#ifdef UNIFORMITY_INFO same @@ +236,5 @@ > +#ifdef UNIFORMITY_INFO > +static void > +printUniformityInfo(UserInputData& aData) > +{ > + char * touchAction; nit: char* touchAction. @@ +602,5 @@ > , mKeyDownCount(0) > , mTouchEventsFiltered(false) > , mKeyEventsFiltered(false) > + { > +#ifdef UNIFORMITY_INFO Same, just make the default false and initialize it later. We can only read preferences on the main thread. Does the GeckoInputDispatcher occur only on the main thread? @@ +655,5 @@ > int mKeyDownCount; > bool mTouchEventsFiltered; > bool mKeyEventsFiltered; > BitSet32 mTouchDown; > +#ifdef UNIFORMITY_INFO Same, please remove UNIFORMITY_INFO ifdefs. @@ +770,5 @@ > nsEventStatus status = nsEventStatus_eIgnore; > if (action != AMOTION_EVENT_ACTION_HOVER_MOVE) { > bool captured; > status = sendTouchEvent(data, &captured); > +#ifdef UNIFORMITY_INFO Same @@ +771,5 @@ > if (action != AMOTION_EVENT_ACTION_HOVER_MOVE) { > bool captured; > status = sendTouchEvent(data, &captured); > +#ifdef UNIFORMITY_INFO > + if(mEnabledUniformityInfo) Please put printUniformity in brackets {}. Thanks!

(In reply to Jinho Hwang [:jeffhwang] from comment #16) > Created attachment 8420791 [details] [diff] [review] > Uniformity.patch > > (In reply to Benoit Girard (:BenWa) from comment #15) > > I've been rolling some of this into the profiler to make this easier to use. > > But I don't track input events: > > Wow, great ~ :) it's nice to have frame uniformity tool inside of profiler. > > One question about the tool, the graph on your blog looks like representing > FPS. > If the dots are near 16ms, that means 60FPS. > Is there any extra information for movements of layers? > If it provides that informations, it will be much useful thought.. > > Anyway, I'll just keep this patch here for the project Slik in progress. I haven't blogged about layer position uniformity but it was recently added after the blog post: https://www.dropbox.com/s/r45t7h1c2qa42i5/Screenshot%202014-05-12%2014.46.10.png I don't track the layer position so we could have you patch report that information to the profiler to add this feature in as well.

Thanks mason for the review. (In reply to Mason Chang [:mchang] from comment #17) > Mostly looks good. Please fix the nits and ask for review again. Asking you for review again after fixing the nits you point. > We can only read > preferences on the main thread. Does the GeckoInputDispatcher occur only on > the main thread? Yes, It does running on main thread so far. (In reply to Benoit Girard (:BenWa) from comment #18) > I haven't blogged about layer position uniformity but it was recently added > > after the blog post: > https://www.dropbox.com/s/r45t7h1c2qa42i5/Screenshot%202014-05-12%2014.46.10.png Thanks BenWa for the detail info. It looks great :) wow~ Profiler is getting improved. > I don't track the layer position so we could have you patch report that > information to the profiler to add this feature in as well. How about the touch information? are you willing to add the touch information as well?

(In reply to Jinho Hwang [:jeffhwang] from comment #19) > > I don't track the layer position so we could have you patch report that > > information to the profiler to add this feature in as well. > > How about the touch information? are you willing to add the touch > information as well? I mean I do track layer position but I don't track touch information.

Comment on attachment 8423742 [details] [diff] [review] UniformityInfo.patch Review of attachment 8423742 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, but I'm not a module owner. Asking :benwa for review as well. ::: gfx/layers/composite/ContainerLayerComposite.cpp @@ +236,5 @@ > } > > +static void PrintUniformityInfo(Layer* aLayer) > +{ > + nit: remove line break.

r+'d. What happens next?

I have to rebase for mozilla-central. Sorry for letting it bitrot.

Rebased for mozilla-central tip. Try build: https://tbpl.mozilla.org/?tree=Try&rev=cec658b2a513

Looks like a good try build for b2g.