Bug 66283 – [Qt][WK2] Add pixel test support

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Balazs Kelemen]

Implement the missing stuff in the Qt port as a first step.

[Balazs Kelemen]

Created attachment 104016 [details]
Patch

Most of the platform independent results are matching

[Balazs Kelemen]

Created attachment 104017 [details]
Patch

Copyrights missed from the previous patch

[Andreas Kling]

Comment on attachment 104017 [details]
Patch

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

> Source/WebKit2/WebKit2API.pri:93
> +    $$SOURCE_DIR/WebKit2/Shared/API/c/WKImageQt.h \

This path isn't correct.

> Tools/WebKitTestRunner/qt/TestInvocationQt.cpp:42
> +    const QByteArray &data = buffer.data();

Style, & placement.

> Tools/WebKitTestRunner/qt/TestInvocationQt.cpp:49
> +    const char *ptr = data.data();

Style, * placement.

> Tools/WebKitTestRunner/qt/TestInvocationQt.cpp:67
> +        hash.addData(reinterpret_cast<const char*>(image.scanLine(row)), image.width() * 4);

You could use QImage::constScanLine() here to avoid calling detach().
"image.width() * 4" isn't necessarily correct, you should use QImage::bytesPerLine().

[Balazs Kelemen]

Created attachment 105498 [details]
Patch

[Balazs Kelemen]

(In reply to comment #3)
> (From update of attachment 104017 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=104017&action=review
> 
> > Source/WebKit2/WebKit2API.pri:93
> > +    $$SOURCE_DIR/WebKit2/Shared/API/c/WKImageQt.h \
> 
> This path isn't correct.

Fixed.

> 
> > Tools/WebKitTestRunner/qt/TestInvocationQt.cpp:42
> > +    const QByteArray &data = buffer.data();
> 
> Style, & placement.
> 
> > Tools/WebKitTestRunner/qt/TestInvocationQt.cpp:49
> > +    const char *ptr = data.data();
> 
> Style, * placement.
> 

Fixed.

> > Tools/WebKitTestRunner/qt/TestInvocationQt.cpp:67
> > +        hash.addData(reinterpret_cast<const char*>(image.scanLine(row)), image.width() * 4);

Fixed. With constScanLine I had to add a QImage::copy call in WKImageCreateQImage to make it work (not crash in memcpy). It is ok since it matches with the CG implementation.

> 
> You could use QImage::constScanLine() here to avoid calling detach().
> "image.width() * 4" isn't necessarily correct, you should use QImage::bytesPerLine().

Fixed.

[Andreas Kling]

Comment on attachment 105498 [details]
Patch

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

Come on, man :)

> Source/WebKit2/WebKit2API.pri:-78
> -    $$SOURCE_DIR/WebKit2/Shared/API/c/WKImage.h \

This file is still there.

> Source/WebKit2/WebKit2API.pri:92
> +    $$SOURCE_DIR/WebKit2/Shared/API/c/qt/WKImage.h \

This file does not exist.

> Tools/ChangeLog:1
> [phone]  Balazs Kelemen  <[e-mail]>

Double ChangeLog entries.

[Balazs Kelemen]

Created attachment 105644 [details]
Patch

[WebKit Review Bot]

Comment on attachment 105644 [details]
Patch

Rejecting attachment 105644 [details] from commit-queue.

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

Last 500 characters of output:
ba44d0e4e1ac9c236f9df69307c13e15e9e2b3c4
r94489 = 9578b85a8725dcf68f34ad20d585e385773fa87e
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

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

[Balazs Kelemen]

Committed r94524: <http://trac.webkit.org/changeset/94524>