Bug 75777 – [Qt] Enable CSS_FILTERS in Qt build

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Noam Rosenthal]

Also, we need to disable hardware-acceleration of filters for Qt by default, until those are supported in TextureMapperGL.

[Noam Rosenthal]

Created attachment 121560 [details]
Patch

[Igor Trindade Oliveira]

Comment on attachment 121560 [details]
Patch

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

> Source/WebCore/ChangeLog:6


nit: Enable instead of Enabls

> Source/WebCore/Target.pri:3858
> +contains(DEFINES, ENABLE_CSS_FILTERS=1) {

I think we do not need this, FilterEffectRenderer.* are already protected by ifdef guards. We can add both files directly and we do not need to pollute the target.pri.

[Noam Rosenthal]

Created attachment 121563 [details]
Patch, revised with Igor's comments.

[Noam Rosenthal]

Created attachment 121569 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 121569 [details]
Patch

why are there pixel tests for invalid parsing?

[Noam Rosenthal]

(In reply to comment #5)
> (From update of attachment 121569 [details])
> why are there pixel tests for invalid parsing?

Good point, I'll remove them.

[Noam Rosenthal]

Created attachment 121581 [details]
Patch, removed some unnecessary png results.

[WebKit Review Bot]

Comment on attachment 121581 [details]
Patch, removed some unnecessary png results.

Clearing flags on attachment: 121581

Committed r104403: <http://trac.webkit.org/changeset/104403>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

It broke 15 tests on Qt 4.8 and zillion tests on Qt 5 (WK1 and WK2 too):
- http://build.webkit.org/results/Qt%20Linux%20Release/r104403%20%2841863%29/results.html
- http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20-%20Qt5-WebKit1/r104403%20(2049)
- http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r104403%20%2818171%29/results.html

I rolled it out, because now bots exit early (after 500 fails) and now the test coverage is 0 % because of this patch. I think you guys should have run layout tests before landing as all other developers do or watch the bots after landing. Please be more careful next time. Thanks.

[Noam Rosenthal]

(In reply to comment #10)
> It broke 15 tests on Qt 4.8 and zillion tests on Qt 5 (WK1 and WK2 too):
> - http://build.webkit.org/results/Qt%20Linux%20Release/r104403%20%2841863%29/results.html
> - http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20-%20Qt5-WebKit1/r104403%20(2049)
> - http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r104403%20%2818171%29/results.html
> 
> I rolled it out, because now bots exit early (after 500 fails) and now the test coverage is 0 % because of this patch. I think you guys should have run layout tests before landing as all other developers do or watch the bots after landing. Please be more careful next time. Thanks.

This is apparently not a problem with the bot/build-system, that didn't trigger a clean build.

[Noam Rosenthal]

Comment on attachment 121581 [details]
Patch, removed some unnecessary png results.

Committing again, might have to force a clean to the build bot.

[WebKit Review Bot]

Comment on attachment 121581 [details]
Patch, removed some unnecessary png results.

Clearing flags on attachment: 121581

Committed r104421: <http://trac.webkit.org/changeset/104421>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

Reopen, because Noam rolled out it again: http://trac.webkit.org/changeset/104425

[Csaba Osztrogonác]

Comment on attachment 121581 [details]
Patch, removed some unnecessary png results.

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

The blocker build-webkit clean build bug was fixed. I'll update the expected results and land the patch immediately.

> Source/WebCore/Target.pri:1173
> +    rendering/FilterEffectRenderer.cpp \

It will conflict with http://trac.webkit.org/changeset/104447/trunk/Source/WebCore/Target.pri,
because you added this file to Target.pri.

> Source/WebCore/Target.pri:2288
> +    rendering/FilterEffectRenderer.h \

ditto

[Csaba Osztrogonác]

Patch landed in http://trac.webkit.org/changeset/104560 with fixed layout test results.

[Noam Rosenthal]

(In reply to comment #17)
> Patch landed in http://trac.webkit.org/changeset/104560 with fixed layout test results.

Thanks Ossy, you're a hero.