Bug 33750

Summary: Add back an AffineTransform class for use by SVG
Product: WebKit Reporter: Sam Weinig <sam>
Component: SVGAssignee: Dirk Schulze <krit>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, eric, gustavo, krit, mitz, oliver, simon.fraser, tkent, webkit.review.bot, xan.lopez, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Stress test
none
reimplementation of AffineTransformation
none
reimplementation of AffineTransformation
none
reimplementation of AffineTransformation
none
reimplementation of AffineTransformation
none
Add back AffineTransform for SVG
none
Add back AffineTransform for SVG none

Description Sam Weinig 2010-01-15 16:55:39 PST
SVG does not need the full generality of the TransformationMatrix class and could benefit from the AffineTransform class being added back and being used by SVG.
Comment 1 Simon Fraser (smfr) 2010-01-15 16:59:25 PST
Do you have any performance data to show that TransformationMatrix is an issue?
Comment 2 Sam Weinig 2010-01-15 17:32:04 PST
Created attachment 46721 [details]
Stress test

We came to this conclusion looking at the attached stress test with 50,000 rects.  It seems every RenderPath has at least one TransformationMatrix.
Comment 3 Dirk Schulze 2010-01-17 09:26:23 PST
It is planned to introduce 3D transformations to SVG 2.0 (http://www.w3.org/TR/2009/WD-SVG-Transforms-20090320/). Even if we concentrate on SVG 1.1 at the moment, we might want to implement it later. Thus we should let TransformationMatrix in and try to optimize the use of it. olliej did allready a great step forward with bug 33766 and bug 30055.
Comment 4 Dirk Schulze 2010-01-30 11:37:58 PST
Created attachment 47771 [details]
reimplementation of AffineTransformation

After a discussion on IRC, all agreed to reimplement AffineTransform in a platform independent way. This is the first and implements AffineTransform and let Canvas make use of it. Next patches will introduce AffineTransform to SVG.
Comment 5 WebKit Review Bot 2010-01-30 11:42:56 PST
Attachment 47771 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
bCore/platform/graphics/transforms/AffineTransform.cpp:74:  More than one command on the same line  [whitespace/newline] [4]
WebCore/platform/graphics/transforms/AffineTransform.cpp:75:  More than one command on the same line  [whitespace/newline] [4]
WebCore/platform/graphics/transforms/AffineTransform.cpp:76:  More than one command on the same line  [whitespace/newline] [4]
WebCore/platform/graphics/transforms/AffineTransform.cpp:119:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:120:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:130:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:136:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/qt/TransformationMatrixQt.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cg/TransformationMatrixCG.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/html/canvas/CanvasRenderingContext2D.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/platform/graphics/wx/TransformationMatrixWx.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/html/canvas/CanvasRenderingContext2D.cpp:369:  Missing spaces around /  [whitespace/operators] [3]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:26:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:50:  cairo_matrix_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:54:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/transforms/AffineTransform.h:30:  Should have a space between // and comment  [whitespace/comments] [4]
WebCore/platform/graphics/transforms/AffineTransform.h:126:  Missing spaces around *=  [whitespace/operators] [3]
WebCore/platform/graphics/transforms/AffineTransform.h:143:  cairo_matrix_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 27


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 WebKit Review Bot 2010-01-30 12:01:04 PST
Attachment 47771 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/222034
Comment 7 Dirk Schulze 2010-01-31 11:27:05 PST
Created attachment 47795 [details]
reimplementation of AffineTransformation

build fix and some fixes of the style issues mentioned by the bot.
Comment 8 WebKit Review Bot 2010-01-31 11:31:32 PST
Attachment 47795 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
ontextSkia.cpp:821:  One space before end of line comments  [whitespace/comments] [5]
WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:822:  One space before end of line comments  [whitespace/comments] [5]
WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:823:  One space before end of line comments  [whitespace/comments] [5]
WebCore/platform/graphics/skia/TransformationMatrixSkia.cpp:31:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/PathCairo.cpp:333:  c_matrix is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/transforms/AffineTransform.cpp:118:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:119:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:129:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:135:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/qt/TransformationMatrixQt.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cg/TransformationMatrixCG.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/wx/TransformationMatrixWx.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:26:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:50:  cairo_matrix_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:54:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/transforms/AffineTransform.h:130:  Missing spaces around *=  [whitespace/operators] [3]
WebCore/platform/graphics/transforms/AffineTransform.h:147:  cairo_matrix_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 19


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 WebKit Review Bot 2010-01-31 11:38:52 PST
Attachment 47795 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/223435
Comment 10 Dirk Schulze 2010-01-31 11:52:34 PST
Created attachment 47797 [details]
reimplementation of AffineTransformation

grrr... typo in GraphicsContextSkia
Comment 11 WebKit Review Bot 2010-01-31 12:01:06 PST
Attachment 47797 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
ontextSkia.cpp:821:  One space before end of line comments  [whitespace/comments] [5]
WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:822:  One space before end of line comments  [whitespace/comments] [5]
WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:823:  One space before end of line comments  [whitespace/comments] [5]
WebCore/platform/graphics/skia/TransformationMatrixSkia.cpp:31:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/PathCairo.cpp:333:  c_matrix is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/transforms/AffineTransform.cpp:118:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:119:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:129:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:135:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/qt/TransformationMatrixQt.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cg/TransformationMatrixCG.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/wx/TransformationMatrixWx.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:26:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:50:  cairo_matrix_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:54:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/transforms/AffineTransform.h:130:  Missing spaces around *=  [whitespace/operators] [3]
WebCore/platform/graphics/transforms/AffineTransform.h:147:  cairo_matrix_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 19


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Dirk Schulze 2010-01-31 12:41:51 PST
(In reply to comment #11)
> Attachment 47797 [details] did not pass style-queue:

I won't fix the mentioned style issues with the following reasons:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> Last 3072 characters of output:
> ontextSkia.cpp:821:  One space before end of line comments 
> [whitespace/comments] [5]
> WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:822:  One space before
> end of line comments  [whitespace/comments] [5]
> WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:823:  One space before
> end of line comments  [whitespace/comments] [5]
Looks like a style decission of the chromium guys and it makes sense here for a better code reading.

> WebCore/platform/graphics/skia/TransformationMatrixSkia.cpp:31:  Found other
> header before a header this file implements. Should be: config.h, primary
> header, blank line, and then alphabetically sorted.  [build/include_order] [4]
AffineTransform is also a primary header. The names of TransformationMatrix*.cpp might change to AffineTransform*.cpp later.

> WebCore/platform/graphics/cairo/PathCairo.cpp:333:  c_matrix is incorrectly
> named. Don't use underscores in your identifier names.  [readability/naming]
Gtk port uses underscores for variables.

> [4]
> WebCore/platform/graphics/transforms/AffineTransform.cpp:118:  Tests for
> true/false, null/non-null, and zero/non-zero should all be done without
> equality comparisons.  [readability/comparison_to_zero] [5]
> WebCore/platform/graphics/transforms/AffineTransform.cpp:119:  Tests for
> true/false, null/non-null, and zero/non-zero should all be done without
> equality comparisons.  [readability/comparison_to_zero] [5]
> WebCore/platform/graphics/transforms/AffineTransform.cpp:129:  Tests for
> true/false, null/non-null, and zero/non-zero should all be done without
> equality comparisons.  [readability/comparison_to_zero] [5]
> WebCore/platform/graphics/transforms/AffineTransform.cpp:135:  Tests for
> true/false, null/non-null, and zero/non-zero should all be done without
> equality comparisons.  [readability/comparison_to_zero] [5]
It's necessary to check for 0 here, since values are allowed to be negative.

> WebCore/platform/graphics/qt/TransformationMatrixQt.cpp:27:  Found other header
> before a header this file implements. Should be: config.h, primary header,
> blank line, and then alphabetically sorted.  [build/include_order] [4]
see above

> WebCore/platform/graphics/cg/TransformationMatrixCG.cpp:27:  Found other header
> before a header this file implements. Should be: config.h, primary header,
> blank line, and then alphabetically sorted.  [build/include_order] [4]
see above

> WebCore/platform/graphics/wx/TransformationMatrixWx.cpp:27:  Found other header
> before a header this file implements. Should be: config.h, primary header,
> blank line, and then alphabetically sorted.  [build/include_order] [4]
see above

> WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:26:  Found other
> header before a header this file implements. Should be: config.h, primary
> header, blank line, and then alphabetically sorted.  [build/include_order] [4]
see above

> WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:50: 
> cairo_matrix_t is incorrectly named. Don't use underscores in your identifier
> names.  [readability/naming] [4]
Style guide of the gtk port.

> WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:54:  Extra space
> before ( in function call  [whitespace/parens] [4]
Style decission of the gtk port.

> WebCore/platform/graphics/transforms/AffineTransform.h:130:  Missing spaces
> around *=  [whitespace/operators] [3]
Found it that way in all other headers for operator like FloatPoint, Size, Rect as well as TransformationMatrix and others. Maybe also a false positive?

> WebCore/platform/graphics/transforms/AffineTransform.h:147:  cairo_matrix_t is
> incorrectly named. Don't use underscores in your identifier names. 
> [readability/naming] [4]
see above

> Total errors found: 19
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.
Comment 13 Adam Barth 2010-01-31 13:52:13 PST
> Looks like a style decission of the chromium guys and it makes sense here for a
> better code reading.

I can't speak to the other issues, but the chromium port is intended to be 100% in compliance with the WebKit style guide.  Any deviations are errors and should be corrected as convenient.
Comment 14 Dirk Schulze 2010-01-31 14:19:48 PST
(In reply to comment #13)
> > Looks like a style decission of the chromium guys and it makes sense here for a
> > better code reading.
> 
> I can't speak to the other issues, but the chromium port is intended to be 100%
> in compliance with the WebKit style guide.  Any deviations are errors and
> should be corrected as convenient.

Did you take a look at the affected code part? I have no problem to fix it with an upcomming patch, but it makes more sense to have more spaces here.
Comment 15 Adam Barth 2010-01-31 14:30:30 PST
> Did you take a look at the affected code part? I have no problem to fix it with
> an upcomming patch, but it makes more sense to have more spaces here.

Situations like that are actually called out specifically in the Google Style Guide (which agrees with the WebKit Style Guide on this point).

I don't mean to be the style enforcer.  I mostly wanted to make that comment for future reference.  Other platforms (e.g., Gtk, Qt) do use different style in parts of WebKit to match their respective platforms.  In the case of Chromium, we've changed the platform to match WebKit style (which is one reason it took a while to upstream the "glue" layer).

If you'd like to remove the whitespace, that would be great.  If you feel like other things are more deserving of your attention, that's fine too.  :)
Comment 16 Dirk Schulze 2010-01-31 14:34:54 PST
(In reply to comment #15)
> > Did you take a look at the affected code part? I have no problem to fix it with
> > an upcomming patch, but it makes more sense to have more spaces here.
> 
> Situations like that are actually called out specifically in the Google Style
> Guide (which agrees with the WebKit Style Guide on this point).
> 
> I don't mean to be the style enforcer.  I mostly wanted to make that comment
> for future reference.  Other platforms (e.g., Gtk, Qt) do use different style
> in parts of WebKit to match their respective platforms.  In the case of
> Chromium, we've changed the platform to match WebKit style (which is one reason
> it took a while to upstream the "glue" layer).
> 
> If you'd like to remove the whitespace, that would be great.  If you feel like
> other things are more deserving of your attention, that's fine too.  :)

Ok, thanks for the clarification. :-)
Comment 17 Dirk Schulze 2010-01-31 14:36:02 PST
Comment on attachment 47797 [details]
reimplementation of AffineTransformation

Clearing review flag. Patch landed in http://trac.webkit.org/changeset/54112
Comment 18 Kent Tamura 2010-01-31 22:00:54 PST
(In reply to comment #17)
> (From update of attachment 47797 [details])
> Clearing review flag. Patch landed in http://trac.webkit.org/changeset/54112

WebCore.vcproj missed a </File> line, and Windows build has the following error even if we fixed WebCore.vcproj.  We'll roll this back.

7>   Creating library e:\cg\src\third_party\WebKit\WebKitBuild\lib\WebKit.lib an
d object e:\cg\src\third_party\WebKit\WebKitBuild\lib\WebKit.exp
7>LINK : warning LNK4199: /DELAYLOAD:QuartzCore.dll ignored; no imports found fr
om QuartzCore.dll
7>LINK : warning LNK4199: /DELAYLOAD:QuartzCoreInterface.dll ignored; no imports
 found from QuartzCoreInterface.dll
7>WebCore_debug.lib(GraphicsContextCG.obj) : error LNK2019: unresolved external
symbol "public: void __thiscall WebCore::GraphicsContextPlatformPrivate::concatC
TM(class WebCore::AffineTransform const &)" (?concatCTM@GraphicsContextPlatformP
rivate@WebCore@@QAEXABVAffineTransform@2@@Z) referenced in function "public: voi
d __thiscall WebCore::GraphicsContext::concatCTM(class WebCore::AffineTransform
const &)" (?concatCTM@GraphicsContext@WebCore@@QAEXABVAffineTransform@2@@Z)
7>e:\cg\src\third_party\WebKit\WebKitBuild\bin\WebKit.dll : fatal error LNK1120:
 1 unresolved externals
Comment 19 Dirk Schulze 2010-01-31 23:47:27 PST
Created attachment 47815 [details]
reimplementation of AffineTransformation

fixes windows build.
Comment 20 WebKit Review Bot 2010-01-31 23:53:19 PST
Attachment 47815 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
ontextSkia.cpp:821:  One space before end of line comments  [whitespace/comments] [5]
WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:822:  One space before end of line comments  [whitespace/comments] [5]
WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:823:  One space before end of line comments  [whitespace/comments] [5]
WebCore/platform/graphics/skia/TransformationMatrixSkia.cpp:31:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/PathCairo.cpp:333:  c_matrix is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/transforms/AffineTransform.cpp:118:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:119:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:129:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/transforms/AffineTransform.cpp:135:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/qt/TransformationMatrixQt.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cg/TransformationMatrixCG.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/wx/TransformationMatrixWx.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:26:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:50:  cairo_matrix_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/platform/graphics/cairo/TransformationMatrixCairo.cpp:54:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/platform/graphics/transforms/AffineTransform.h:130:  Missing spaces around *=  [whitespace/operators] [3]
WebCore/platform/graphics/transforms/AffineTransform.h:147:  cairo_matrix_t is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 19


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Dirk Schulze 2010-02-01 00:08:32 PST
(In reply to comment #20)
> Attachment 47815 [details] did not pass style-queue:

See comment https://bugs.webkit.org/show_bug.cgi?id=33750#c12 .
Comment 22 Eric Seidel (no email) 2010-02-01 16:12:02 PST
Attachment 47815 [details] was posted by a committer and has review+, assigning to Dirk Schulze for commit.
Comment 23 Dirk Schulze 2010-02-07 16:59:53 PST
Created attachment 48307 [details]
Add back AffineTransform for SVG

The complete patch to get AffineTransform back in action. I also upload the patch to check if it builds on all platforms. I'll let the r? online, if builds pass.
Comment 24 Dirk Schulze 2010-02-07 17:02:01 PST
Comment on attachment 47815 [details]
reimplementation of AffineTransformation

patch landed in r54137.
Comment 25 WebKit Review Bot 2010-02-07 17:23:13 PST
Attachment 48307 [details] did not pass style-queue:

Failed to run "['git', 'reset', '--hard', 'HEAD']" exit_code: 128
fatal: Could not reset index file to revision 'HEAD'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 WebKit Review Bot 2010-02-07 17:45:24 PST
Attachment 48307 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/244189
Comment 27 Adam Barth 2010-02-07 18:23:02 PST
Sorry about the botspam.  I think the bots were having some trouble.
Comment 28 Dirk Schulze 2010-02-08 06:15:15 PST
(In reply to comment #26)
> Attachment 48307 [details] did not build on gtk:
> Build output: http://webkit-commit-queue.appspot.com/results/244189

I'm pretty sure, that it works on gtk. (Tested it with trunk on my own system)

I also run check-webkit-style. I got  warnings, all about == 0 checks. They were found in AffineTransform and are neccessary there.
Comment 29 Dirk Schulze 2010-02-08 12:06:55 PST
Created attachment 48349 [details]
Add back AffineTransform for SVG

update was needed after Nikos DRT patch.
Comment 30 WebKit Review Bot 2010-02-08 12:11:05 PST
Attachment 48349 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/transforms/AffineTransform.h:120:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/platform/graphics/skia/ImageSkia.cpp:33:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/svg/SVGTransformDistance.cpp:254:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 3


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 31 Dirk Schulze 2010-02-08 12:12:57 PST
(In reply to comment #30)
> Attachment 48349 [details] did not pass style-queue:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> WebCore/platform/graphics/transforms/AffineTransform.h:120:  Tests for
> true/false, null/non-null, and zero/non-zero should all be done without
> equality comparisons.  [readability/comparison_to_zero] [5]
> WebCore/platform/graphics/skia/ImageSkia.cpp:33:  Found other header before a
> header this file implements. Should be: config.h, primary header, blank line,
> and then alphabetically sorted.  [build/include_order] [4]
> WebCore/svg/SVGTransformDistance.cpp:254:  Tests for true/false, null/non-null,
> and zero/non-zero should all be done without equality comparisons. 
> [readability/comparison_to_zero] [5]
> Total errors found: 3
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.
see also comment 28
Comment 32 Nikolas Zimmermann 2010-02-08 12:22:03 PST
Comment on attachment 48349 [details]
Add back AffineTransform for SVG

Looks fine, r=me.
Please change the static_cast<float> to narrowPrecisionToFloat, as discussed on IRC.
Comment 33 Dirk Schulze 2010-02-08 14:15:35 PST
Comment on attachment 48349 [details]
Add back AffineTransform for SVG

landed in http://trac.webkit.org/changeset/54503 . Build fixes for chromium and win were added in 54504, 54509