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

Too much time spent drawing the chrome #1990

Closed
jrmuizel opened this issue Nov 5, 2017 · 19 comments
Closed

Too much time spent drawing the chrome #1990

jrmuizel opened this issue Nov 5, 2017 · 19 comments

Comments

@jrmuizel
Copy link
Contributor

@jrmuizel jrmuizel commented Nov 5, 2017

I see 44 draw calls on this yaml file:
https://jrmuizel.github.io/webrender/chrome.zip

It would be good to get a diagnosis of what's going on.

https://bugzilla.mozilla.org/show_bug.cgi?id=1414679

@glennw
Copy link
Member

@glennw glennw commented Nov 5, 2017

I will look into this as part of the batching investigation I'm looking at this week.

@glennw
Copy link
Member

@glennw glennw commented Nov 5, 2017

One thing I've noticed that is causing a lot of batch breaks:

The favicon images are added with NEAREST texture filtering instead of LINEAR.

The texture cache currently just allocates NEAREST images as standalone textures, on the assumption that these will be very rare.

We can make the texture cache a bit smarter about this. Is it intentional / required that the favicon images use NEAREST filter though? I would have though having them as LINEAR would be a better result?

That doesn't explain all the issues - but it's the first thing I noticed.

@glennw
Copy link
Member

@glennw glennw commented Nov 5, 2017

Forcing those images to be nearest drops the draw call count from 44 to 38 - still not great. I'll investigate the rest as part of #1984.

@mstange
Copy link
Contributor

@mstange mstange commented Nov 5, 2017

Yes, favicons using NEAREST is intentional, see for example bug 1041845 and this testcase from it.

@glennw
Copy link
Member

@glennw glennw commented Nov 5, 2017

OK, I've opened #1992 to make that case efficient. It's a small amount of work - I'll try to do it this week in between work on other tasks.

@mstange
Copy link
Contributor

@mstange mstange commented Nov 6, 2017

I should probably mention that the browser CSS only asks for NEAREST on favicons if the UI scale is exactly 2. So if you're testing optimizations for this case in Firefox on a machine that doesn't have a scale of exactly 2, you might want to set the layout.css.devPixelsPerPx pref to 2 in order to reproduce it.

@kvark
Copy link
Member

@kvark kvark commented Nov 6, 2017

Note that strictly speaking we don't have to separate NEAREST from LINEAR textures. We could snap the texture coordinate in the fragment shader to the nearest texel center instead. That, if batch breaks are really killing us :)

@glennw
Copy link
Member

@glennw glennw commented Nov 6, 2017

Around half of the GPU time is spent applying opacity filters which are not necessary. There's two cases that we need to handle here:

  • The iframe has a large rect, with opacity(0). We're not detecting this correctly right now, due to some implementation details related to how property bindings are processed (even though they aren't actually used here).
  • Most of the images are a stacking context with an opacity filter, but contain only a single item. Instead of drawing these to a render target, and then compositing with a filter, they should be drawn direct with the opacity applied as a color to the image.

The planned work #1774 (comment) and below will cover these cases. I'm working towards those now.

There are a few other items of interest that I need to look into:

  • The borders are being drawn as "complex" borders, but they look like they should be hitting the simple border path.
  • The rectangles are taking longer to draw than expected. They may be going through the alpha pass for some reason. I need to investigate this further.
@glennw
Copy link
Member

@glennw glennw commented Nov 6, 2017

Oh, another issue we have is that all the image masks for the tab text get blitted from the texture cache into a clip mask before being applied. In the simple case (where the only clip is the image mask), we should just sample directly from the image mask in the texture cache. That will save quite a lot of GPU time in this test case.

@glennw
Copy link
Member

@glennw glennw commented Nov 6, 2017

Note to self: there are also several very large standalone textures in the texture cache. They look mostly redundant - they are a single color. Perhaps redundant image masks? Any ideas @jrmuizel ? I'll look into them in detail anyway.

@jrmuizel
Copy link
Contributor Author

@jrmuizel jrmuizel commented Nov 6, 2017

@glennw how did you identify those textures?

@glennw
Copy link
Member

@glennw glennw commented Nov 6, 2017

@jrmuizel If you run wrench with the texture cache debugger (and my pending patch to support favicons in the texture cache), you'll see several images shown that don't go in the texture cache. The only reason they shouldn't go in the cache is if they are > 512 pixels. I haven't looked any further than that yet.

This is a picture of the texture cache debugger when viewing this scene. Generally, you just see the four pages of the texture cache shown here. All of the images shown here on the left appear to be large, standalone textures (unless I'm mis-interpreting the debugger output).

tc

@jrmuizel
Copy link
Contributor Author

@jrmuizel jrmuizel commented Nov 6, 2017

Ok, three of those are likely:
1509921165-img-1.png: PNG image data, 2494 x 66, 8-bit/color RGBA, non-interlaced
1509921165-img-3.png: PNG image data, 2494 x 148, 8-bit/color RGBA, non-interlaced
1509921165-img-4.png: PNG image data, 2494 x 66, 8-bit/color RGBA, non-interlaced

Which are solid color blob images coming from the Chrome. I can investigate why.

@glennw
Copy link
Member

@glennw glennw commented Nov 6, 2017

Yep, that seems like them. I think they are being rendered as normal Image primitives too. I thought they would be image masks, but I don't think they are.

@glennw
Copy link
Member

@glennw glennw commented Nov 6, 2017

Yep, they are attached to Image primitives in the yaml file - would be interesting to see where they come from.

@glennw
Copy link
Member

@glennw glennw commented Nov 6, 2017

Most border primitives are falling off the fast path, but they shouldn't be. The code doesn't correctly handle the case where some edges are solid, and other edges are none/hidden as a fast path border.

@glennw
Copy link
Member

@glennw glennw commented Nov 6, 2017

There's also a significant amount of time drawing clip mask(s) and solid rects. The clip mask(s) should be resolved by drawing them in segments and/or a fast path for uniform radii. Need to investigate the solid rectangle time - it might just be that there is a full screen rect being drawn (in which case, the z-buffer will reject that on a normal page where there is content).

@jrmuizel
Copy link
Contributor Author

@jrmuizel jrmuizel commented Nov 7, 2017

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1415001 about the large images. It shouldn't be too hard to do better.

@gw3583
Copy link
Collaborator

@gw3583 gw3583 commented Aug 2, 2018

This is no longer an issue - drawing the chrome is a very small part of the CPU/GPU profile now.

@gw3583 gw3583 closed this Aug 2, 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
5 participants
You can’t perform that action at this time.