Mac/gpu: Don't show uninitialized surfaces while resizing plugins / composited tabs.

Mac/gpu: Don't show uninitialized surfaces while resizing plugins / composited tabs.

BUG=53165
TEST=Go to http://webkit.org/blog/386/3d-transforms/. Resize window. Watch a youtube video click the "make bigger" button. No garbage should appear. There are still some funky artifacts at the top/right border during resizing caused by clamping the texture instead of filling with white, and sometimes (rarely) the tab flashes black/white but I'd like to tackle that in follow-up CLs. This is already much better than what's in the tree currently.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=65220

[stuartmorgan, 2010-10-28 23:32:21]

It seems really hacky to have the size changing to the new size, but the surface
still the old size; we're just trading one kind of artifact for another. I think
if we go down this pass we're going to end up with layers of bandaids.

Isn't the real fix for this going to have to be at the view level, having the
view wait to resize, pass on the new ID, etc. until there's content? Or,
alternately, requiring that surfaces be painted to before the "new surface"
message is sent? We could certainly do the latter (force a layer render during
resize handling) quite easily in the plugins at least.

[Nico, 2010-10-29 04:38:45]

On Thu, Oct 28, 2010 at 4:32 PM,  <stuartmorgan@chromium.org> wrote:
> It seems really hacky to have the size changing to the new size, but the
> surface
> still the old size; we're just trading one kind of artifact for another. I
> think
> if we go down this pass we're going to end up with layers of bandaids.

My plan is to draw the IOSurface only at its real size and to fill the
extra space on the right and bottom with white – like the rwhvmac does
it. I admit I only thought about the compositor case, for which this
is perfect. For plugins that take up the whole tab, it's also great,
but for in-page plugins that change their size it might be suboptimal.
But then again, these are pretty rare.

This is simple and imho natural (since it's what rwhvmac does too). If
you want, I can put that into this CL.

> Isn't the real fix for this going to have to be at the view level, having
> the view wait to resize, pass on the new ID, etc. until there's content? Or,
> alternately, requiring that surfaces be painted to before the "new surface"
> message is sent? We could certainly do the latter (force a layer render
> during
> resize handling) quite easily in the plugins at least.
>
> http://codereview.chromium.org/4101002/show
>

[stuartmorgan, 2010-11-02 20:31:23]

On 2010/10/29 04:38:45, Nico wrote:
> I admit I only thought about the compositor case, for which this
> is perfect. For plugins that take up the whole tab, it's also great

Fair enough. I do think adding the white paint now makes more sense than doing
the incremental changes with a different set of artifacts.

[Nico, 2010-11-05 17:23:03]

On 2010/11/02 20:31:23, stuartmorgan wrote:
> On 2010/10/29 04:38:45, Nico wrote:
> > I admit I only thought about the compositor case, for which this
> > is perfect. For plugins that take up the whole tab, it's also great
> 
> Fair enough. I do think adding the white paint now makes more sense than doing
> the incremental changes with a different set of artifacts.

Done.

[stuartmorgan, 2010-11-05 17:35:26]

http://codereview.chromium.org/4101002/diff/19001/20005
File chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc
(right):

http://codereview.chromium.org/4101002/diff/19001/20005#newcode103
chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc:103:
glClearColor(255, 255, 255, 255);
This needs to be conditional on the surface being opaque, with the transparent
version still using 0, 0, 0, 0. We have an opaque flag at this layer, so we can
do it correctly now and not have this bite us later when we get transparent
support.

(I don't remember whether we set opaque for plugins right now; if not we should
change that and put a TODO there.)

[Nico, 2010-11-05 17:54:10]

On Fri, Nov 5, 2010 at 1:35 PM,  <stuartmorgan@chromium.org> wrote:
>
> http://codereview.chromium.org/4101002/diff/19001/20005
> File
> chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc
> (right):
>
> http://codereview.chromium.org/4101002/diff/19001/20005#newcode103
> chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc:103:
> glClearColor(255, 255, 255, 255);
> This needs to be conditional on the surface being opaque, with the
> transparent version still using 0, 0, 0, 0. We have an opaque flag at
> this layer, so we can do it correctly now and not have this bite us
> later when we get transparent support.

The |opaque_| flag here means "replace alpha channel with 1.0 before
drawing" and is only set for pepper 3d plugins.

The right way forward is probably to
a) set opaque for all plugins except a hardcoded whitelist (earth)
b) if opaque isn't set, don't use the hole cutting stuff
c) get rid of the "if(opaque)" code in
accelerated_surface_container.cc, since things are opaque by default

That should go into a different CL. If I put in the |if (opaque)
clear(255, 255, 255, 255) else clear(0, 0, 0, 0)| into this cl, then
window resizing will show a black (from the surface) and white (from
the rwhvmac) border until that other CL lands. So I'd prefer to not
put it in now.

[stuartmorgan, 2010-11-05 18:03:09]

Okay, I didn't realize that opaque didn't mean what we wanted here. LGTM then,
but I do want to make sure handling transparent plugins doesn't get pushed out
so far that we build up a so many little breakages like this this that
untangling them all will be painful.