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

Reduce the amount of extra work needed for hit testing #2807

Closed
kvark opened this issue Jun 5, 2018 · 10 comments
Closed

Reduce the amount of extra work needed for hit testing #2807

kvark opened this issue Jun 5, 2018 · 10 comments

Comments

@kvark
Copy link
Member

@kvark kvark commented Jun 5, 2018

#2797 enabled rendering on async scene rebuilds. AFAIK, hit-testing should only need the results of build_layer_screen_rects_and_cull_layers function, and don't actually care about the batches and render task graph. Thus, we can save a few cycles here.

cc @staktrace @nical @gw3583

@staktrace
Copy link
Contributor

@staktrace staktrace commented Jun 5, 2018

That seems reasonable to me. I thought it would be complicated to extract the bits of the render process that hit-testing needs, but if it's straightforward then that would be a nice optimization.

And just to be explicit, we we need to ensure that this "slimmed down render" at the very least does the sampling (currently here, applies those messages (which are going to be ScrollNodeWithId and AppendDynamicProperties messages), before going on to update the clip scroll tree and rebuild the hit-tester.

staktrace added a commit to staktrace/webrender that referenced this issue Jun 19, 2018
This path just updates the clip scroll tree and hit-tester, without
doing all the extra render work needed for a full frame render. When we
do an async scene swap, we can use this instead of doing a full render
to get the hit-test information in sync with Gecko.

Fixes servo#2807
@staktrace
Copy link
Contributor

@staktrace staktrace commented Jun 20, 2018

I wrote a patch for this and on the firefox talos perf tests it improved perf by a bit on displaylist_mutate but regressed tresize a bunch, so I need to look into it a bit more.

staktrace added a commit to staktrace/webrender that referenced this issue Jun 25, 2018
This path just updates the clip scroll tree and hit-tester, without
doing all the extra render work needed for a full frame render. When we
do an async scene swap, we can use this instead of doing a full render
to get the hit-test information in sync with Gecko.

Fixes servo#2807
staktrace added a commit to staktrace/webrender that referenced this issue Jun 25, 2018
This path just updates the clip scroll tree and hit-tester, without
doing all the extra render work needed for a full frame render. When we
do an async scene swap, we can use this instead of doing a full render
to get the hit-test information in sync with Gecko.

Fixes servo#2807
staktrace added a commit to staktrace/webrender that referenced this issue Jun 27, 2018
This path just updates the clip scroll tree and hit-tester, without
doing all the extra render work needed for a full frame render. When we
do an async scene swap, we can use this instead of doing a full render
to get the hit-test information in sync with Gecko.

Fixes servo#2807
staktrace added a commit to staktrace/webrender that referenced this issue Jul 4, 2018
This path just updates the clip scroll tree and hit-tester, without
doing all the extra render work needed for a full frame render. When we
do an async scene swap, we can use this instead of doing a full render
to get the hit-test information in sync with Gecko.

Fixes servo#2807
staktrace added a commit to staktrace/webrender that referenced this issue Jul 26, 2018
This path just updates the clip scroll tree and hit-tester, without
doing all the extra render work needed for a full frame render. When we
do an async scene swap, we can use this instead of doing a full render
to get the hit-test information in sync with Gecko.

Fixes servo#2807
@jrmuizel
Copy link
Contributor

@jrmuizel jrmuizel commented Jul 26, 2018

Do we know why?

@staktrace
Copy link
Contributor

@staktrace staktrace commented Jul 26, 2018

When I investigated it previously I came to the conclusion that the first render happens before the vsync and makes the second render (post-vsync) cheaper. Visualization is in https://bugzilla.mozilla.org/show_bug.cgi?id=1478566#c9

I'm not sure if the same thing is still happening now; I can check to see if it is.

staktrace added a commit to staktrace/webrender that referenced this issue Jul 26, 2018
This path just updates the clip scroll tree and hit-tester, without
doing all the extra render work needed for a full frame render. When we
do an async scene swap, we can use this instead of doing a full render
to get the hit-test information in sync with Gecko.

Fixes servo#2807
@staktrace
Copy link
Contributor

@staktrace staktrace commented Jul 27, 2018

I looked at it again today and found a number of different issues. But the important one was bug 1479075. I'll do another talos run with the fix for that.

staktrace added a commit to staktrace/webrender that referenced this issue Jul 30, 2018
This path just updates the clip scroll tree and hit-tester, without
doing all the extra render work needed for a full frame render. When we
do an async scene swap, we can use this instead of doing a full render
to get the hit-test information in sync with Gecko.

Fixes servo#2807
@staktrace
Copy link
Contributor

@staktrace staktrace commented Sep 26, 2018

This should be fixed by #3052 (which got reverted and relanded as #3092)

@kvark
Copy link
Member Author

@kvark kvark commented Sep 26, 2018

Fixed by #3092

@kvark kvark closed this Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.