Bug 66283 - [Qt][WK2] Add pixel test support
Summary: [Qt][WK2] Add pixel test support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Balazs Kelemen
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2011-08-16 01:23 PDT by Balazs Kelemen
Modified: 2011-09-05 03:53 PDT (History)
3 users (show)

See Also:


Attachments
Patch (5.81 KB, patch)
2011-08-16 01:35 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff
Patch (7.58 KB, patch)
2011-08-16 02:14 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff
Patch (9.19 KB, patch)
2011-08-29 10:09 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff
Patch (8.43 KB, patch)
2011-08-30 09:53 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Balazs Kelemen 2011-08-16 01:23:06 PDT
Implement the missing stuff in the Qt port as a first step.
Comment 1 Balazs Kelemen 2011-08-16 01:35:35 PDT
Created attachment 104016 [details]
Patch

Most of the platform independent results are matching
Comment 2 Balazs Kelemen 2011-08-16 02:14:09 PDT
Created attachment 104017 [details]
Patch

Copyrights missed from the previous patch
Comment 3 Andreas Kling 2011-08-29 08:46:14 PDT
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().
Comment 4 Balazs Kelemen 2011-08-29 10:09:55 PDT
Created attachment 105498 [details]
Patch
Comment 5 Balazs Kelemen 2011-08-29 10:14:43 PDT
(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.
Comment 6 Andreas Kling 2011-08-30 07:36:03 PDT
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
> +2011-08-29  Balazs Kelemen  <kbalazs@webkit.org>

Double ChangeLog entries.
Comment 7 Balazs Kelemen 2011-08-30 09:53:49 PDT
Created attachment 105644 [details]
Patch
Comment 8 WebKit Review Bot 2011-09-03 09:22:03 PDT
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
Comment 9 Balazs Kelemen 2011-09-05 03:52:39 PDT
Committed r94524: <http://trac.webkit.org/changeset/94524>