Summary: | Flash of white on left and right edges of screen when showing fullscreen controller when movie doesn't fill the entire screen | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||||
Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | ||||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Jer Noble
2011-06-10 16:25:00 PDT
Created attachment 96822 [details]
Patch
Comment on attachment 96822 [details]
Patch
Obseleting. Does not actually fix the flash.
Created attachment 96837 [details]
Patch
Created attachment 96845 [details]
Patch
Forgot to set up the bounds of the newly created content layer.
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 ;-) (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 on attachment 96845 [details]
Patch
This looks fine.
Committed r88653: <http://trac.webkit.org/changeset/88653> |