Bug 62491 - Flash of white on left and right edges of screen when showing fullscreen controller when movie doesn't fill the entire screen
Summary: Flash of white on left and right edges of screen when showing fullscreen cont...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-10 16:25 PDT by Jer Noble
Modified: 2011-06-13 11:02 PDT (History)
0 users

See Also:


Attachments
Patch (1.43 KB, patch)
2011-06-10 16:28 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (7.83 KB, patch)
2011-06-10 19:45 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (7.85 KB, patch)
2011-06-10 23:10 PDT, Jer Noble
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2011-06-10 16:25:00 PDT
Flash of white on left and right edges of screen when showing fullscreen controller when movie doesn't fill the entire screen
Comment 1 Jer Noble 2011-06-10 16:28:38 PDT
Created attachment 96822 [details]
Patch
Comment 2 Jer Noble 2011-06-10 17:02:27 PDT
Comment on attachment 96822 [details]
Patch

Obseleting.  Does not actually fix the flash.
Comment 3 Jer Noble 2011-06-10 19:45:58 PDT
Created attachment 96837 [details]
Patch
Comment 4 Jer Noble 2011-06-10 23:10:37 PDT
Created attachment 96845 [details]
Patch

Forgot to set up the bounds of the newly created content layer.
Comment 5 Darin Adler 2011-06-11 09:06:26 PDT
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 ;-)
Comment 6 Jer Noble 2011-06-11 13:43:09 PDT
(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. ;)
Comment 7 Simon Fraser (smfr) 2011-06-12 16:23:40 PDT
Comment on attachment 96845 [details]
Patch

This looks fine.
Comment 8 Jer Noble 2011-06-13 11:02:25 PDT
Committed r88653: <http://trac.webkit.org/changeset/88653>