Bug 62491 – Flash of white on left and right edges of screen when showing fullscreen controller when movie doesn't fill the entire screen

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Flash of white on left and right edges of screen when showing fullscreen controller when movie doesn't fill the entire screen

[Jer Noble]

Created attachment 96822 [details]
Patch

[Jer Noble]

Comment on attachment 96822 [details]
Patch

Obseleting.  Does not actually fix the flash.

[Jer Noble]

Created attachment 96837 [details]
Patch

[Jer Noble]

Created attachment 96845 [details]
Patch

Forgot to set up the bounds of the newly created content layer.

[Darin Adler]

Comment on attachment 96845 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96845&action=review

Looks great to me, but you said Simon should review it. I am setting review+ but I am not Simon!

> Source/WebCore/rendering/RenderLayerBacking.cpp:314
> +        // Special case the RenderFullScreen case because we know its
> +        // style does not come from CSS and it is therefore safe to set
> +        // the layer's background color to the renderer's style's background
> +        // color.
> +        updateBackgroundColor();

This comment says why it’s safe to use updateBackgroundColor here, but not why it’s important to use it (to avoid flashes you would see if we used a tiled layer).

Also, the statement “its style does not come from CSS” does not clearly express to me why it’s safe to use the background color. It’s tantalizingly close to saying why, but doesn’t quite make it clear.

Also, that orphaned word color bugs me ;-)

[Jer Noble]

(In reply to comment #5)
> (From update of attachment 96845 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=96845&action=review
> 
> Looks great to me, but you said Simon should review it. I am setting review+ but I am not Simon!
> 
> > Source/WebCore/rendering/RenderLayerBacking.cpp:314
> > +        // Special case the RenderFullScreen case because we know its
> > +        // style does not come from CSS and it is therefore safe to set
> > +        // the layer's background color to the renderer's style's background
> > +        // color.
> > +        updateBackgroundColor();
> 
> This comment says why it’s safe to use updateBackgroundColor here, but not why it’s important to use it (to avoid flashes you would see if we used a tiled layer).
> 
> Also, the statement “its style does not come from CSS” does not clearly express to me why it’s safe to use the background color. It’s tantalizingly close to saying why, but doesn’t quite make it clear.
> 
> Also, that orphaned word color bugs me ;-)

New comment:

        // RenderFullScreen renderers have no content, and only a solid
        // background color.  They also can be large enough to trigger the
        // creation of a tiled-layer, which can cause flashing problems
        // during repainting.  Special case the RenderFullScreen case because
        // we know its style does not come from CSS and it is therefore will
        // not contain paintable content (e.g. background images, gradients,
        // etc.), so it's safe to set the layer's background color to the 
        // renderer's style's background color.

Now "color's" less lonely. ;)

[Simon Fraser (smfr)]

Comment on attachment 96845 [details]
Patch

This looks fine.

[Jer Noble]

Committed r88653: <http://trac.webkit.org/changeset/88653>