Bug 98315 – Turn Compositing on by default in WebKit build

• Component: CSS
• Status: RESOLVED FIXED

[Rik Cabanier]

Compositing is behind a define that is currently turned off in WebKit builds.

Simon Fraser recommended that it should be turned on.

[Rik Cabanier]

Created attachment 166985 [details]
first try

[Simon Fraser (smfr)]

Comment on attachment 166985 [details]
first try

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

> Source/WebCore/Configurations/FeatureDefines.xcconfig:51
>  ENABLE_CSS_IMAGE_ORIENTATION = ;
>  ENABLE_CSS_IMAGE_RESOLUTION = ;
>  ENABLE_CSS_REGIONS = ENABLE_CSS_REGIONS;
>  ENABLE_CSS_SHADERS = ENABLE_CSS_SHADERS;
> -ENABLE_CSS_COMPOSITING = ;
> +ENABLE_CSS_COMPOSITING = ENABLE_CSS_COMPOSITING;
>  ENABLE_CSS_STICKY_POSITION = ENABLE_CSS_STICKY_POSITION;
>  ENABLE_CSS_VARIABLES = ;
>  ENABLE_CSS3_TEXT_DECORATION = ;

You need to do this in all the FeatureDefines.xcconfig files, and make sure ENABLE_CSS_COMPOSITING is included in the FEATURE_DEFINES lines.

[Build Bot]

Comment on attachment 166985 [details]
first try

Attachment 166985 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14132815

[Rik Cabanier]

Created attachment 166991 [details]
touched .in files

[WebKit Review Bot]

Attachment 166991 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/ChangeLog:23:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Rik Cabanier]

Created attachment 166994 [details]
updated all featuresDefines files

[Simon Fraser (smfr)]

Comment on attachment 166994 [details]
updated all featuresDefines files

Bad patch

[WebKit Review Bot]

Attachment 166994 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/ChangeLog:23:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Rik Cabanier]

Created attachment 167000 [details]
forgot to update patch

[Rik Cabanier]

Created attachment 167021 [details]
tried changing expectations for chromium

[WebKit Review Bot]

Attachment 167021 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
LayoutTests/ChangeLog:16:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Rik Cabanier]

Created attachment 167023 [details]
fixed patch

[WebKit Review Bot]

Comment on attachment 167000 [details]
forgot to update patch

Attachment 167000 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14171011

[WebKit Review Bot]

Comment on attachment 167000 [details]
forgot to update patch

Attachment 167000 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14152347

New failing tests:
css3/compositing/blend-mode-property.html
css3/compositing/should-have-compositing-layer.html

[Simon Fraser (smfr)]

Comment on attachment 167023 [details]
fixed patch

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

> Source/JavaScriptCore/ChangeLog:9
> +        smfr said that this feature should be turned on and that browser can elect to
> +        pick it up

A better comment would be "enable this feature on trunk." I don't think any further justification is required, but you might want to reference a bug or bugs about enabling it on other platforms.

> Source/WebCore/css/CSSPropertyNames.in:8
>  //
> +
>  // Microsoft extensions are documented here:

Whitespace

> Source/WebCore/css/CSSValueKeywords.in:45
>  all
> +
>  //

Whitespace

[Rik Cabanier]

Created attachment 167121 [details]
updated comments and removed whitespace

[Build Bot]

Comment on attachment 167121 [details]
updated comments and removed whitespace

Attachment 167121 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14181052

[Rik Cabanier]

Created attachment 167126 [details]
dummy change to .in file

[Simon Fraser (smfr)]

Comment on attachment 167126 [details]
dummy change to .in file

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

> LayoutTests/ChangeLog:9
> +        smfr said that this feature should be turned on and that browser can elect to
> +        pick it up. Existing test are now passing. New baselines submitted.

Has old comment.

[Rik Cabanier]

Created attachment 167143 [details]
patched changelog

[WebKit Review Bot]

Comment on attachment 167143 [details]
patched changelog

Rejecting attachment 167143 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ted in Chromium rebaselines after r130411.
Auto-merging Source/JavaScriptCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Chromium rebaselines after r130411.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.

Full output: http://queues.webkit.org/results/14176247

[WebKit Review Bot]

Comment on attachment 167143 [details]
patched changelog

Rejecting attachment 167143 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
LICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 IndexedDB: promote objectstore/index backend ids to the frontend

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.

Full output: http://queues.webkit.org/results/14168509

[Rik Cabanier]

Created attachment 167224 [details]
bots couldn't merge changelogs

[Rik Cabanier]

Created attachment 167229 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 167229 [details]
Patch for landing

Clearing flags on attachment: 167229

Committed r130460: <http://trac.webkit.org/changeset/130460>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Roger Fong]

Hello, can I go ahead and enable this on Windows too? Shouldn't cause any issues right?

[Rik Cabanier]

(In reply to comment #27)
> Hello, can I go ahead and enable this on Windows too? Shouldn't cause any issues right?

No, feel free to do so.
Will all the CALayer and CG code work there too?

[Roger Fong]

(In reply to comment #28)
> (In reply to comment #27)
> > Hello, can I go ahead and enable this on Windows too? Shouldn't cause any issues right?
> 
> No, feel free to do so.
> Will all the CALayer and CG code work there too?

Should be fine yeah
Thanks

[Roger Fong]

Just kidding.
Simon says that the blend stuff uses CIFilters and on Windows there are no CIFilters.
So that won't work at all.
I think I'll just keep blending disabled and skip the tests on Windows.

[Dean Jackson]

We'll be able to do some of the operations on Windows using a back door. It will require code though.