Bug 80632 - Move TransformationMatrix and TransformState to LayoutUnits.
Summary: Move TransformationMatrix and TransformState to LayoutUnits.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Levi Weintraub
URL:
Keywords:
Depends on:
Blocks: 60318
  Show dependency treegraph
 
Reported: 2012-03-08 13:16 PST by Levi Weintraub
Modified: 2012-03-09 13:04 PST (History)
7 users (show)

See Also:


Attachments
Patch (8.37 KB, patch)
2012-03-08 13:47 PST, Levi Weintraub
no flags Details | Formatted Diff | Diff
Patch for landing (9.84 KB, patch)
2012-03-08 15:54 PST, Levi Weintraub
no flags Details | Formatted Diff | Diff
Patch for landing (10.89 KB, patch)
2012-03-09 11:04 PST, Levi Weintraub
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Levi Weintraub 2012-03-08 13:16:35 PST
We want to preserve the added precision we'll get in the render tree when switching to FractionalLayoutUnits through transformations. To this end, I propose moving TransformationMatrix and TransformState to operate on LayoutUnits instead of integers.
Comment 1 Simon Fraser (smfr) 2012-03-08 13:33:22 PST
I assume you mean mapRect() and clampedBoundsOfProjectedQuad() in TransformationMatrix (the rest is in doubles).

TransformState has no use of IntRect so I'm confused there. You mean HitTestingTransformState?
Comment 2 Levi Weintraub 2012-03-08 13:47:17 PST
Created attachment 130897 [details]
Patch
Comment 3 WebKit Review Bot 2012-03-08 13:51:19 PST
Attachment 130897 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/platform/graphics/transforms/TransformState.h:83:  The parameter name "y" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Levi Weintraub 2012-03-08 14:22:43 PST
Comment on attachment 130897 [details]
Patch

Thanks for the review, Simon.
Comment 5 Early Warning System Bot 2012-03-08 14:53:22 PST
Comment on attachment 130897 [details]
Patch

Attachment 130897 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11894159
Comment 6 Levi Weintraub 2012-03-08 15:04:29 PST
(In reply to comment #5)
> (From update of attachment 130897 [details])
> Attachment 130897 [details] did not pass qt-ews (qt):
> Output: http://queues.webkit.org/results/11894159

I've found the build issue with this. I'll upload a fixed patch once this goes through the remaining EWS bots.
Comment 7 Gyuyoung Kim 2012-03-08 15:23:37 PST
Comment on attachment 130897 [details]
Patch

Attachment 130897 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11906133
Comment 8 Levi Weintraub 2012-03-08 15:54:13 PST
Created attachment 130927 [details]
Patch for landing
Comment 9 WebKit Review Bot 2012-03-08 15:58:01 PST
Attachment 130927 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/transforms/TransformState.h:83:  The parameter name "y" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Eric Seidel (no email) 2012-03-08 16:27:34 PST
Comment on attachment 130897 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130897&action=review

>> Source/WebCore/platform/graphics/transforms/TransformState.h:83
>> +    void move(LayoutUnit x, LayoutUnit y, TransformAccumulation = FlattenTransform);
> 
> The parameter name "y" adds no information, so it should be removed.  [readability/parameter_name] [5]

Bug in check-webkit-style.  I bet it thinks 'y' is a substring of LayoutUnit.
Comment 11 WebKit Review Bot 2012-03-08 21:23:51 PST
Comment on attachment 130927 [details]
Patch for landing

Rejecting attachment 130927 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
orm.a(out/Debug/obj.target/Source/WebCore/WebCore.gyp/../../../webcore_platform/Source/WebCore/platform/graphics/transforms/TransformationMatrix.o): in function WebCore::TransformationMatrix::mapRect(WebCore::FractionalLayoutRect const&) const:Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:646: error: undefined reference to 'WebCore::enclosingFractionalLayoutRect(WebCore::FloatRect const&)'
collect2: ld returned 1 exit status
make: *** [out/Debug/webkit_unit_tests] Error 1

Full output: http://queues.webkit.org/results/11892345
Comment 12 Levi Weintraub 2012-03-09 11:04:30 PST
Created attachment 131064 [details]
Patch for landing
Comment 13 WebKit Review Bot 2012-03-09 11:14:35 PST
Attachment 131064 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/transforms/TransformState.h:83:  The parameter name "y" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 WebKit Review Bot 2012-03-09 13:04:44 PST
Comment on attachment 131064 [details]
Patch for landing

Clearing flags on attachment: 131064

Committed r110322: <http://trac.webkit.org/changeset/110322>
Comment 15 WebKit Review Bot 2012-03-09 13:04:49 PST
All reviewed patches have been landed.  Closing bug.