Bug 51687

Summary: [Qt] GraphicsContext::getCTM() does not need to make a copy of QPainter::combinedTransform()
Product: WebKit Reporter: Jan Erik Hanssen <jhanssen>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ariya.hidayat, commit-queue, eric, kling, webkit.review.bot
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch none

Jan Erik Hanssen
Reported 2010-12-28 13:36:39 PST
GraphicsContext::getCTM() does not need to make a copy of the QTransform returned by QPainter::combinedTransform(), it could instead use a const reference. This will avoid a copy for compilers that don't implement (or enable by default) the NRVO optimization.
Attachments
Patch (1.49 KB, patch)
2010-12-28 13:39 PST, Jan Erik Hanssen 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jan Erik Hanssen
Comment 1 2010-12-28 13:39:56 PST
Created attachment 77575 [details] Patch
Eric Seidel (no email)
Comment 2 2010-12-28 13:43:58 PST
Comment on attachment 77575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77575&action=review > WebCore/platform/graphics/qt/GraphicsContextQt.cpp:292 > return AffineTransform(matrix.m11(), matrix.m12(), matrix.m21(), Why don't we just have a AffineTransform(const QTransform&) constructor which is compiled on Qt?
Eric Seidel (no email)
Comment 3 2010-12-28 13:45:42 PST
I guess that's not the pattern. No other platfrom seems to have custom constructors anymore. Strange, since that's how we (at least used to) do IntPoint, IntRect, etc.
WebKit Commit Bot
Comment 4 2010-12-28 14:56:12 PST
Comment on attachment 77575 [details] Patch Clearing flags on attachment: 77575 Committed r74728: <http://trac.webkit.org/changeset/74728>
WebKit Commit Bot
Comment 5 2010-12-28 14:56:19 PST
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 6 2010-12-28 16:13:32 PST
http://trac.webkit.org/changeset/74728 might have broken Leopard Intel Debug (Tests)
Note You need to log in before you can comment on or make changes to this bug.