WebTransformationMatrix is going to become the compositor's data type for transforms. It currently wraps WebCore::TransformationMatrix, and in the future there will be additional implementation of the same interface. However, transformation math is a high profile, high risk black hole for errors... there is a lot of "almost-redundant" code that can have tiny mistakes that are very difficult to find. So it is probably a good idea to be very aggressive with testing here, going for very tight, specific, and complete test coverage. This patch will provide all of those tests, except for the mapRect/mapQuad/projectQuad routines that will eventually be removed. The new mapRect/mapQuad/projectQuad functionality would be in CCMathUtil, and testing does need to be added to that code soon. Then, after the dust settles from GTFO, WebTransformationMatrix and CCMathUtil could probably be merged.
Created attachment 141843 [details] Patch Note: I also skipped the blend() function for now. Will talk to vollick@ about what coverge he feels exists or is needed before spending time on that.
(In reply to comment #1) > Created an attachment (id=141843) [details] > Patch > > Note: I also skipped the blend() function for now. Will talk to vollick@ about what coverge he feels exists or is needed before spending time on that. Spoke with him, and we will need to add test coverage for blend(), I would like to do it in a separate patch. Thanks!
Comment on attachment 141843 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141843&action=review > Source/WebKit/chromium/tests/WebTransformationMatrixTest.cpp:234 > + WebTransformationMatrix B = A; This isn't assignment, it's copy-initialization via the copy constructor. The assignment needs to be in a separate statement. Also, maybe you should test the return value from the assignment operator, e.g. A = B = C.
Created attachment 142048 [details] Addressed reviewer comment
Comment on attachment 142048 [details] Addressed reviewer comment R=me. I don't quite follow why blend needs to be in a separate patch, but this set of tests looks fine to me.
Comment on attachment 142048 [details] Addressed reviewer comment There was no strong reason, please feel free to cancel the cq if you want them to be together for any reason. blend() will add another 7-8 tests. I'm running into challenges trying to figure out why the blending is not as "linear" as I expect for skewY; and testing rotation blending will not be ideal either since they use quaternions; I'll probably just resort to testing the status quo in a few different configurations.
Comment on attachment 142048 [details] Addressed reviewer comment Clearing flags on attachment: 142048 Committed r117199: <http://trac.webkit.org/changeset/117199>
All reviewed patches have been landed. Closing bug.
(In reply to comment #6) > (From update of attachment 142048 [details]) > There was no strong reason, please feel free to cancel the cq if you want them to be together for any reason. > > blend() will add another 7-8 tests. I'm running into challenges trying to figure out why the blending is not as "linear" as I expect for skewY; and testing rotation blending will not be ideal either since they use quaternions; I'll probably just resort to testing the status quo in a few different configurations. https://bugs.webkit.org/show_bug.cgi?id=87066 will add unit tests for blend().