Re: Issue 123388 in chromium: RenderSurface for opacity with descendentDrawsContent is slow when large

[chro...@googlecode.com]

Comment #4 on issue 123388 by jam...@chromium.org: RenderSurface for
opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

Currently whenever we use a render surface we render into a texture. We
also always track damage on a per-RS basis. I think it'd be fine to simply
not delete the texture unless it received damage or we are over/close to
some memory limit, in the worst case it means that we don't free up the
texture as quickly.

[chro...@googlecode.com]

Comment #5 on issue 123388 by pi...@chromium.org: RenderSurface for opacity

with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

OK - the request from the UI people was mainly to be able to kill the
layers that are underneath the "frozen" layer (which would cause more
damage to happen).
We don't necessarily need this for this particular case, but we'll need to
change other parts of the UI to guarantee that we won't generate damage
(e.g. freeze the renderer somehow).

[chro...@googlecode.com]

Updates:
Status: Assigned
Owner: pi...@chromium.org

Comment #6 on issue 123388 by sain...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

Antoine: please triage, set mstone, etc...

[chro...@googlecode.com]

Updates:
Blockedon: 99556

Comment #7 on issue 123388 by dan...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

99556 is a bug I had made before to cache surface contents.

[chro...@googlecode.com]

Updates:
Labels: -Pri-2 -Feature-Ash Pri-1 Feature-Ash-WM Mstone-21

Comment #10 on issue 123388 by pi...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

Must have for 21.

[chro...@googlecode.com]

Updates:
Owner: zlie...@chromium.org

Comment #12 on issue 123388 by bac...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

Assigning to zlieber@ as a starter bug. He's going to work on a minor
refactor or two to get up to speed with workflow/codebase first --- so not
started, just assigned for now.

[chro...@googlecode.com]

Comment #15 on issue 123388 by zlie...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

Issue 99556 has been merged into this issue.

[chro...@googlecode.com]

Comment #16 on issue 123388 by zlie...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

Plan of attack:

1. Changes in layers.

Changes in layers properties need to be divided into two groups: those that
change content of the target surface, and thereby invalidate any cached
quads, and those that don't. Currently we only have one boolean flag in the
layer, m_layerPropertyChanged. We need to add a second flag, named
m_layerSurfacePropertyChanged (NAME TBD) and set it in appropriate
operations instead of the other one.

2. Changes in CCDamageTracker

We need to change the semantics of m_currentDamageRect. If our surface's
owning layer did not change its property (m_layerPropertyChanged) and the
rest of the layers are spotless clean, then m_currentDamageRect will be
empty. ("Spotless" means none
of the two flags is set). If the rest is not spotless clean (just clean, or
dirty), m_currentDamageRect will describe the extent of the change.
Exception is the root surface, which will always be either dirty or
spotless - otherwise we'll end up with empty rootDamageRect.

3. Changes in CCRenderSurfaceDrawQuad

Each CCRenderSurfaceDrawQuad will now store a pointer to a render surface
that it represents. It will get the pointer in the constructor.

4. Changes in appendQuads

All quads are always appened, regardless of the m_currentDamageRect state
(as long as they are in the scissorRect). However, if the
m_currentDamageRect is empty, the surface quad gets an extra flag
- !m_contentsChanged. This flag will be later used for removing unneeded
render passes.

5. Changes in render passes

The culling of the extra passes will be done on the RenderPass level, after
it has been assembled. In the future, it will be done after the render
passes are combined with other passes in a different process.

The alg for the culling is as follows:

- Iterate through RenderPasses in the order OPPOSITE to drawing, i.e. start
from the root.
- For each CCRenderSurfaceDrawQuad, do the following
- If its contents haven't changed AND the we have a valid texture:
- Recursively remove all render passes that contribute to this surface.
For each render pass being removed, find all CCRenderSurfaceDrawQuads in it
and kill their render passes too. To find the render passes, search through
the entire list to find the one that has a same render surface pointer as
the CCRenderSurfaceDrawQuad.

[chro...@googlecode.com]

Updates:
Cc: wjmacl...@chromium.org

Comment #17 on issue 123388 by zlie...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

(No comment was entered for this change.)

[chro...@googlecode.com]

Updates:
Labels: WebKit-ID-88482

Comment #18 on issue 123388 by zlie...@chromium.org: RenderSurface for

[chro...@googlecode.com]

Updates:
Labels: -WebKit-ID-88482 WebKit-ID-88482-UNCONFIRMED

Comment #19 on issue 123388 by bugdro...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large

http://code.google.com/p/chromium/issues/detail?id=123388#c19

https://bugs.webkit.org/show_bug.cgi?id=88482

[chro...@googlecode.com]

Comment #21 on issue 123388 by wiltz...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

@zlieber, M21 branch is coming up on Monday. How are you feeling about this
making it or not? Thx,

[chro...@googlecode.com]

Comment #22 on issue 123388 by zlie...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

The patch is ready, I'll try to land it tomorrow; however the effect on the
minimize speed is less than expected. I'm still checking this.

[chro...@googlecode.com]

Updates:
Labels: -WebKit-ID-88482-UNCONFIRMED WebKit-ID-88482-RESOLVED
WebKit-Rev-120539

Comment #23 on issue 123388 by bugdro...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large

http://code.google.com/p/chromium/issues/detail?id=123388#c23

https://bugs.webkit.org/show_bug.cgi?id=88482
http://trac.webkit.org/changeset/120539

[chro...@googlecode.com]

Updates:
Labels: -WebKit-ID-88482-RESOLVED WebKit-ID-88482-UNCONFIRMED
WebKit-Rev-120542

Comment #24 on issue 123388 by bugdro...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large

http://code.google.com/p/chromium/issues/detail?id=123388#c24

https://bugs.webkit.org/show_bug.cgi?id=88482
http://trac.webkit.org/changeset/120542

[chro...@googlecode.com]

Updates:
Labels: -WebKit-ID-88482-UNCONFIRMED WebKit-ID-88482-RESOLVED
WebKit-Rev-120553

Comment #25 on issue 123388 by bugdro...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large

http://code.google.com/p/chromium/issues/detail?id=123388#c25

https://bugs.webkit.org/show_bug.cgi?id=88482
http://trac.webkit.org/changeset/120553

[chro...@googlecode.com]

Updates:
Blockedon: -chromium:99556 chromium:99556

Comment #26 on issue 123388 by dan...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

An update on where this is at:

It seems that not delete/creating the surface texture is a good start (~10%
reduction in time to draw a frame). Not binding and redrawing the contents
of the surface texture adds to this (~10% further reduction).

But the animation is still below 30 fps. There is something else quite
costly in this animation, maybe it is just the blending? We don't have any
data pointing out anything particular yet.

[chro...@googlecode.com]

Updates:
Cc: g...@chromium.org

Comment #27 on issue 123388 by dan...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

To explain away the slow texture creation, piman had some hints to start
with. GLES2DecoderImpl::ClearLevel is going to clear new textures and does
this by mallocing and memsetting a 4M block of memory, then doing
glTexSubImage2D into the texture in 4mb blocks. I'm not sure what we can do
better in this function, maybe not malloc/memset and just keep the 0s
around? Maybe use glClear? </noobface>

In the case of the browser renderer, perhaps we can consider it trusted and
not clear the texture on creation. In ubercomp that would drop the cost on
all surface textures (yay).

[chro...@googlecode.com]

Updates:
Status: Fixed

Comment #28 on issue 123388 by zlie...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

[chro...@googlecode.com]

Updates:
Labels: -WebKit-Rev-120539 -WebKit-Rev-120542 -WebKit-ID-88482-RESOLVED
-WebKit-Rev-120553

Comment #29 on issue 123388 by zlie...@chromium.org: RenderSurface for

[chro...@googlecode.com]

Updates:
Status: Started

Comment #30 on issue 123388 by zlie...@chromium.org: RenderSurface for

[chro...@googlecode.com]

Updates:
Status: Available
Owner: ---

Comment #33 on issue 123388 by zlie...@chromium.org: RenderSurface for

[chro...@googlecode.com]

Comment #34 on issue 123388 by vange...@google.com: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

@zlieber : Would you mind giving an update on the current status of this
bug? Looks like at least some of the proposed work has been done. What's
left to do here and is performance still a problem?

[chro...@googlecode.com]

Comment #35 on issue 123388 by dan...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

Zeev's not working on graphics any more, so I can fill in.

Comment #26/27 give a bit of a summary where this is at. It has dropped off
the radar because the UI stopped animating with opacity < 1 to avoid using
render surfaces.

We are now currently caching surface textures when they are not
dirtied/resized. If the UI wants to do such an animation again, we're going
to have to do some profiling to determine what is the bottleneck.

So, right now, we have no pressure to make this any better and things
are "good enough". If someone would like to make some test cases of bad
situations I'm sure they can find some data to motivate and demonstrate
improvements.

[chro...@googlecode.com]

Comment #36 on issue 123388 by pi...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
http://code.google.com/p/chromium/issues/detail?id=123388

Note: I've heard the UI is doing this again (new maximize animation), but I
don't know if we have this perf issue.

[chro...@googlecode.com]

Updates:
Status: Assigned
Owner: dan...@chromium.org

Comment #40 on issue 123388 by vmi...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large

https://code.google.com/p/chromium/issues/detail?id=123388

Dana, do we still need to do something here or should we archive this?

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[chro...@googlecode.com]

Updates:
Status: Archived

Comment #41 on issue 123388 by dan...@chromium.org: RenderSurface for

opacity with descendentDrawsContent is slow when large
https://code.google.com/p/chromium/issues/detail?id=123388

Archive. There's no strong motivation to do anything here. We even stopped
caching render surface outputs, and just avoid alloc/free each frame right
now.