Bug 34507 - [Qt] Minor improvement to hybrid QPixmap
Summary: [Qt] Minor improvement to hybrid QPixmap
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2010-02-02 18:01 PST by Noam Rosenthal
Modified: 2010-02-18 13:51 PST (History)
2 users (show)

See Also:


Attachments
minor adjustment to hybrid pixmap: allow sending an existing img element as an argument (7.11 KB, patch)
2010-02-02 18:08 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
enable applying a QPixmap from a native QObject to an HTML img element (9.15 KB, patch)
2010-02-11 21:27 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
The native QPixmap/QImage can now assign its value to an existing image element (17.04 KB, patch)
2010-02-12 11:37 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
enable applying a QPixmap from a native QObject to an HTML img element + minor style fixes (17.04 KB, patch)
2010-02-12 13:36 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
The native QPixmap/QImage can now assign its value to an existing image element (17.39 KB, patch)
2010-02-12 17:09 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Noam Rosenthal 2010-02-02 18:01:25 PST
The original patch for hybrid transport of pixmaps between Qt and webkit allows for creating new HTML img elements, but in most cases the HTML image element would already be there. This patch allows adding the QPixmap into an already existing img element without the overhead of toDataUrl, by allowing an optional single parameter to toHTMLImageElement() - an existing img element.
Comment 1 Noam Rosenthal 2010-02-02 18:08:58 PST
Created attachment 47984 [details]
minor adjustment to hybrid pixmap: allow sending an existing img element as an argument

Note: this is coming from a real world use-case where we have a native SDK that exposes images, and we want to use HTML to display them.
Comment 2 Kenneth Rohde Christiansen 2010-02-08 12:11:06 PST
Comment on attachment 47984 [details]
minor adjustment to hybrid pixmap: allow sending an existing img element as an argument

e.g. myQObject.myPixmapProperty.toHTMLImageElement(myHtmlImg);

I don't like this API very much. It is like the to* operated on the myPixmapProperty, but still it takes an image.
Comment 3 Noam Rosenthal 2010-02-08 12:45:22 PST
I see your point. Would something like myQObject.myPixmapProperty.attachToHTMLImageElement(myHtmlImg) be better? Any name is fine with me as long as I can get the functionality in!
Comment 4 Noam Rosenthal 2010-02-11 21:27:17 PST
Created attachment 48620 [details]
enable applying a QPixmap from a native QObject to an HTML img element

Based on comments from Kenneth, renamed toHTMLImageElement to two functions: 
createHTMLImageElement and applyToHTMLImageElement (which takes an existing img element as an argument).
Also added some in-code comments.
Comment 5 Noam Rosenthal 2010-02-12 11:37:12 PST
Created attachment 48653 [details]
The native QPixmap/QImage can now assign its value to an existing image element

Replaced toHTMLImageElement() which creates an element, with assignToHTMLImageElement(element), which takes an existing element as an arg.
Also fixed the style to conform to the webkit style guide.
Comment 6 Kenneth Rohde Christiansen 2010-02-12 11:48:08 PST
Comment on attachment 48653 [details]
The native QPixmap/QImage can now assign its value to an existing image element

Looks fine, but there is at least a minor style issue you could fix before committing.

> +        if (hint == qMetaTypeId<QPixmap >())
Comment 7 Noam Rosenthal 2010-02-12 13:36:50 PST
Created attachment 48659 [details]
enable applying a QPixmap from a native QObject to an HTML img element + minor style fixes

ok - fixed those stray spaces, should be good to go :)
Comment 8 Noam Rosenthal 2010-02-12 17:09:47 PST
Created attachment 48681 [details]
The native QPixmap/QImage can now assign its value to an existing image element

Even better style!
Hope this is sufficient
N
Comment 9 Eric Seidel (no email) 2010-02-17 14:22:46 PST
Comment on attachment 48653 [details]
The native QPixmap/QImage can now assign its value to an existing image element

Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 48653 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 10 WebKit Commit Bot 2010-02-18 13:51:04 PST
Comment on attachment 48681 [details]
The native QPixmap/QImage can now assign its value to an existing image element

Clearing flags on attachment: 48681

Committed r54986: <http://trac.webkit.org/changeset/54986>
Comment 11 WebKit Commit Bot 2010-02-18 13:51:08 PST
All reviewed patches have been landed.  Closing bug.