Bug 24648 - Optimize TransformationMatrix
Summary: Optimize TransformationMatrix
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-17 10:48 PDT by Simon Fraser (smfr)
Modified: 2009-04-04 13:26 PDT (History)
2 users (show)

See Also:


Attachments
Patch, changelog (4.58 KB, patch)
2009-04-02 00:01 PDT, Simon Fraser (smfr)
mitz: review+
Details | Formatted Diff | Diff
Patch to optimize map functions for translations (4.44 KB, patch)
2009-04-02 18:37 PDT, Simon Fraser (smfr)
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2009-03-17 10:48:08 PDT
We should make a pass over TransformationMatrix and optimize it to reduce copying. For example, translate() and translate3d() could easily be optimized. multiply() could be factored with multLeft() to avoid matrix copies, and more.
Comment 1 Simon Fraser (smfr) 2009-04-02 00:01:19 PDT
Created attachment 29183 [details]
Patch, changelog
Comment 2 Simon Fraser (smfr) 2009-04-02 09:26:22 PDT
Comment on attachment 29183 [details]
Patch, changelog

Landed in http://trac.webkit.org/changeset/42172

Keeping bug open for further optimization.
Comment 3 Simon Fraser (smfr) 2009-04-02 18:37:56 PDT
Created attachment 29214 [details]
Patch to optimize map functions for translations
Comment 4 mitz 2009-04-03 11:10:27 PDT
Comment on attachment 29214 [details]
Patch to optimize map functions for translations

> +        return FloatPoint(static_cast<float>(p.x() + m_matrix[3][0]), static_cast<float>(p.y() + m_matrix[3][1]));

I slightly prefer casting the matrix element to float rather than casting the coordinate to double, adding in doubles, and casting the result down to float. r=me either way.
Comment 5 Simon Fraser (smfr) 2009-04-03 11:58:12 PDT
Last patch checked in as http://trac.webkit.org/changeset/42207

I think this is enough for now.
Comment 6 Xan Lopez 2009-04-04 06:25:01 PDT
(In reply to comment #5)
> Last patch checked in as http://trac.webkit.org/changeset/42207
> 
> I think this is enough for now.
> 

     // Like the version above, except that it rounds the mapped point to the nearest integer value.
-    IntPoint mapPoint(const IntPoint&) const;
+    IntPoint mapPoint(const IntPoint& p) const
+    {
+        return roundedIntPoint(mapPoint(p));
+    }

Won't this call itself until the stack blows up? Because I'm getting exactly this with your patch in reddit.com for example.
Comment 7 Simon Fraser (smfr) 2009-04-04 13:12:07 PDT
Ah, I guess it will. I need to call the FloatPoint version.
Comment 8 Simon Fraser (smfr) 2009-04-04 13:26:17 PDT
Fixed mapPoint in http://trac.webkit.org/changeset/42228