Bug 41827

Summary:

[Qt] Canvas putImageData() resets painter state

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

WebCore Misc.

Assignee:

QtWebKit Unassigned <webkit-qt-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, webkit-ews, webkit.review.bot

Priority:

Keywords:

HTML5, Qt, QtTriaged

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch	kling: review-
Proposed patch v2	kling: review-
Proposed patch v3	none
Proposed patch v4	none

Description Andreas Kling 2010-07-07 19:10:59 PDT

In ImageBufferQt.cpp, putImageData() will call end() and begin() on the ImageBufferData's QPainter if painting is active (which it always is, at least for a canvas backend.)

This resets the painter state which is supposed to be in sync with the canvas rendering context's state.

Comment 1 Andreas Kling 2010-07-07 19:50:04 PDT

Created attachment 60828 [details]
Proposed patch

Patch to use QImage instead of QPixmap as the backing store of ImageBuffer.

Comment 2 WebKit Review Bot 2010-07-07 19:52:43 PDT

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

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/qt/ImageBufferData.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 7 files


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

Comment 3 Early Warning System Bot 2010-07-07 19:58:36 PDT

Attachment 60828 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/3496001

Comment 4 Andreas Kling 2010-07-07 20:18:02 PDT

Comment on attachment 60828 [details]
Proposed patch

Oops, used some 4.7+ API here. Leaving r? for visibility. zzz..

Comment 5 Andreas Kling 2010-07-08 04:04:46 PDT

Comment on attachment 60828 [details]
Proposed patch

r- after discussion with Simon. There should be a simpler way to fix this.

Comment 6 Andreas Kling 2010-07-08 04:41:27 PDT

Created attachment 60865 [details]
Proposed patch v2

Better solution - use QPainter::drawImage() to copy pixel data into the canvas backing store.

Comment 7 Andreas Kling 2010-07-08 04:45:45 PDT

Comment on attachment 60865 [details]
Proposed patch v2

r- due to bathroom epiphany. More awesome patch coming shortly.

Comment 8 Andreas Kling 2010-07-08 04:56:29 PDT

Created attachment 60866 [details]
Proposed patch v3

Accomplish the same thing without converting the canvas backing store into a QImage.

Comment 9 Andreas Kling 2010-07-08 05:03:04 PDT

Created attachment 60867 [details]
Proposed patch v4

Patch updated to set correct composition mode when painter isn't active at the time putImageData() is called.

Comment 10 WebKit Commit Bot 2010-07-08 05:38:31 PDT

Comment on attachment 60867 [details]
Proposed patch v4

Clearing flags on attachment: 60867

Committed r62782: <http://trac.webkit.org/changeset/62782>

Comment 11 WebKit Commit Bot 2010-07-08 05:38:41 PDT

All reviewed patches have been landed.  Closing bug.