Bug 33750 – Add back an AffineTransform class for use by SVG

• Component: SVG
• Status: RESOLVED FIXED

[Sam Weinig]

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.

[Simon Fraser (smfr)]

Do you have any performance data to show that TransformationMatrix is an issue?

[Sam Weinig]

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.

[Dirk Schulze]

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.

[Dirk Schulze]

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.

[WebKit Review Bot]

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.

[WebKit Review Bot]

Attachment 47771 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/222034

[Dirk Schulze]

Created attachment 47795 [details]
reimplementation of AffineTransformation

build fix and some fixes of the style issues mentioned by the bot.

[WebKit Review Bot]

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.

[WebKit Review Bot]

Attachment 47795 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/223435

[Dirk Schulze]

Created attachment 47797 [details]
reimplementation of AffineTransformation

grrr... typo in GraphicsContextSkia

[WebKit Review Bot]

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.

[Dirk Schulze]

(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.

[Adam Barth]

> 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.

[Dirk Schulze]

(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.

[Adam Barth]

> 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.  :)

[Dirk Schulze]

(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. :-)

[Dirk Schulze]

Comment on attachment 47797 [details]
reimplementation of AffineTransformation

Clearing review flag. Patch landed in http://trac.webkit.org/changeset/54112

[Kent Tamura]

(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

[Dirk Schulze]

Created attachment 47815 [details]
reimplementation of AffineTransformation

fixes windows build.

[WebKit Review Bot]

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.

[Dirk Schulze]

(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 .

[Eric Seidel (no email)]

Attachment 47815 [details] was posted by a committer and has review+, assigning to Dirk Schulze for commit.

[Dirk Schulze]

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.

[Dirk Schulze]

Comment on attachment 47815 [details]
reimplementation of AffineTransformation

patch landed in r54137.

[WebKit Review Bot]

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.

[WebKit Review Bot]

Attachment 48307 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/244189

[Adam Barth]

Sorry about the botspam.  I think the bots were having some trouble.

[Dirk Schulze]

(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.

[Dirk Schulze]

Created attachment 48349 [details]
Add back AffineTransform for SVG

update was needed after Nikos DRT patch.

[WebKit Review Bot]

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.

[Dirk Schulze]

(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

[Nikolas Zimmermann]

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.

[Dirk Schulze]

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