Bug 64542 - Move TransformState to platform/graphics and give it the option to transform just a FloatQuad
Summary: Move TransformState to platform/graphics and give it the option to transform ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Marrin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-14 11:10 PDT by Chris Marrin
Modified: 2011-07-15 14:48 PDT (History)
2 users (show)

See Also:


Attachments
Patch (46.59 KB, patch)
2011-07-14 15:35 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Patch (46.90 KB, patch)
2011-07-14 15:43 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (6.12 MB, application/zip)
2011-07-14 17:09 PDT, WebKit Review Bot
no flags Details
Patch (81.93 KB, patch)
2011-07-15 11:17 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-03 (6.23 MB, application/zip)
2011-07-15 11:36 PDT, WebKit Review Bot
no flags Details
Revised patch to fix win build problem (82.64 KB, patch)
2011-07-15 12:39 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Patch (81.25 KB, patch)
2011-07-15 13:25 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Patch (76.83 KB, patch)
2011-07-15 13:50 PDT, Chris Marrin
simon.fraser: review+
simon.fraser: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Marrin 2011-07-14 11:10:35 PDT
Move TransformState to platform/graphics and give it the option to transform just a FloatQuad
Comment 1 Chris Marrin 2011-07-14 15:35:30 PDT
Created attachment 100876 [details]
Patch
Comment 2 Chris Marrin 2011-07-14 15:43:29 PDT
Created attachment 100878 [details]
Patch
Comment 3 Simon Fraser (smfr) 2011-07-14 15:53:59 PDT
Comment on attachment 100878 [details]
Patch

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

I think this looks good, but would like to see HitTestingTransformState stay in rendering. Is there a way to share the code but just have a RefCounted subclass?

> Source/WebCore/platform/graphics/transforms/TransformState.cpp:133
> +// HitTestingTransformState methods

I think HitTestingTransformState should either be move back under rendering/, or renamed to RefCountedTransformState or something.

> Source/WebCore/platform/graphics/transforms/TransformState.h:103
> -    bool m_mapQuad;
> +    bool m_mapPoint, m_mapQuad;

Style guidelines recommend having these on separate lines.
Comment 4 WebKit Review Bot 2011-07-14 17:09:11 PDT
Comment on attachment 100878 [details]
Patch

Attachment 100878 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9064394

New failing tests:
fast/repaint/block-selection-gap-in-composited-layer.html
editing/selection/image-before-linebreak.html
editing/selection/replaced-boundaries-3.html
fast/repaint/japanese-rl-selection-repaint.html
Comment 5 WebKit Review Bot 2011-07-14 17:09:21 PDT
Created attachment 100900 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 6 Chris Marrin 2011-07-15 11:17:01 PDT
Created attachment 101009 [details]
Patch
Comment 7 Simon Fraser (smfr) 2011-07-15 11:21:04 PDT
Comment on attachment 101009 [details]
Patch

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

> Source/WebCore/platform/graphics/transforms/TransformState.cpp:29
> +#include <wtf/PassOwnPtr.h>

Don't need this include.

> Source/WebCore/platform/graphics/transforms/TransformState.h:2
> + * Copyright (C) 2009 Apple Inc.  All rights reserved.

+2011

> Source/WebCore/platform/graphics/transforms/TransformState.h:34
> +#include <wtf/PassRefPtr.h>

Not needed?

> Source/WebCore/platform/graphics/transforms/TransformState.h:36
> +#include <wtf/RefCounted.h>

Not needed?

> Source/WebCore/rendering/HitTestingTransformState.cpp:2
> + * Copyright (C) 2009 Apple Inc.  All rights reserved.

Add 2011 too.

> Source/WebCore/rendering/HitTestingTransformState.h:40
> +// So there's really no need for a ref counted version. So This class should be tossed and replaced

s/tossed/removed :)
Comment 8 WebKit Review Bot 2011-07-15 11:36:50 PDT
Comment on attachment 101009 [details]
Patch

Attachment 101009 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9094360

New failing tests:
fast/repaint/block-selection-gap-in-composited-layer.html
editing/selection/image-before-linebreak.html
editing/selection/replaced-boundaries-3.html
fast/repaint/japanese-rl-selection-repaint.html
Comment 9 WebKit Review Bot 2011-07-15 11:36:56 PDT
Created attachment 101014 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 10 Chris Marrin 2011-07-15 12:39:04 PDT
Created attachment 101027 [details]
Revised patch to fix win build problem
Comment 11 Chris Marrin 2011-07-15 13:25:44 PDT
Created attachment 101037 [details]
Patch
Comment 12 Chris Marrin 2011-07-15 13:50:14 PDT
Created attachment 101039 [details]
Patch
Comment 13 Chris Marrin 2011-07-15 14:17:49 PDT
Landed in http://trac.webkit.org/changeset/91110
Comment 14 Simon Fraser (smfr) 2011-07-15 14:48:37 PDT
Comment on attachment 101039 [details]
Patch

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

> Source/WebCore/platform/graphics/transforms/TransformState.cpp:2
> + * Copyright (C) 2009 Apple Inc.  All rights reserved.

+2011

> Source/WebCore/platform/graphics/transforms/TransformState.h:2
> + * Copyright (C) 2009 Apple Inc.  All rights reserved.

+2011

> Source/WebCore/platform/graphics/transforms/TransformState.h:34
> +#include <wtf/PassRefPtr.h>

Remove.

> Source/WebCore/platform/graphics/transforms/TransformState.h:36
> +#include <wtf/RefCounted.h>

Remove.

> Source/WebCore/platform/graphics/transforms/TransformState.h:103
> +    bool m_mapPoint, m_mapQuad;

Separate lines please.