Bug 154100 – [ThreadedCompositor] position:fixed elements do not have their own layers when threaded compositor is enabled.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[ChangSeok Oh]

Created attachment 271043 [details]
a test case

SSIA.
Run the attached test case on threaded compositor enabled webkit build.

[ChangSeok Oh]

Created attachment 271150 [details]
Patch

[Gwang Yoon Hwang]

Comment on attachment 271150 [details]
Patch

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

Nice patch, thanks!
Some nitpiks in the changelog..

> Source/WebKit2/ChangeLog:9
> +        embeded environment. Probably main concern was increasing video memory for fixed elements.

s/embeded/embedded/,
I'm not sure whether is it correct reason or not. As we talked in IRC, it is because the fixed element is not considered to be a new stacking context at first. This feature was added later because of the scrolling performance in the embedded system.

> Source/WebKit2/ChangeLog:10
> +        However, it is time to rethink spliting the elements out to separate layers. Currently the fixed

spliting/splitting
and how about adding a comma after "Currently" ?

> Source/WebKit2/ChangeLog:11
> +        elements are redrawn to a backing store every time layout or scroll happens. It results in bad

Also maybe you missed "the" before bad effect

[ChangSeok Oh]

Comment on attachment 271150 [details]
Patch

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

Thanks for your review!

>> Source/WebKit2/ChangeLog:9
>> +        embeded environment. Probably main concern was increasing video memory for fixed elements.
> 
> s/embeded/embedded/,
> I'm not sure whether is it correct reason or not. As we talked in IRC, it is because the fixed element is not considered to be a new stacking context at first. This feature was added later because of the scrolling performance in the embedded system.

I see. Then Let's get rid of the whole lines.

>> Source/WebKit2/ChangeLog:10
>> +        However, it is time to rethink spliting the elements out to separate layers. Currently the fixed
> 
> spliting/splitting
> and how about adding a comma after "Currently" ?

Yep.

>> Source/WebKit2/ChangeLog:11
>> +        elements are redrawn to a backing store every time layout or scroll happens. It results in bad
> 
> Also maybe you missed "the" before bad effect

Done.

[ChangSeok Oh]

Created attachment 271154 [details]
Patch

[ChangSeok Oh]

Any concern?

[Gwang Yoon Hwang]

I'm okay with this patch. Maybe cgarcia can help you. :)

[ChangSeok Oh]

Comment on attachment 271154 [details]
Patch

Thanks.

[WebKit Commit Bot]

Comment on attachment 271154 [details]
Patch

Clearing flags on attachment: 271154

Committed r196623: <http://trac.webkit.org/changeset/196623>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.