RESOLVED FIXED 33750
Add back an AffineTransform class for use by SVG
https://bugs.webkit.org/show_bug.cgi?id=33750
Summary Add back an AffineTransform class for use by SVG
Sam Weinig
Reported 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.
Attachments
Stress test (1.14 KB, application/xml)
2010-01-15 17:32 PST, Sam Weinig
no flags
reimplementation of AffineTransformation (54.97 KB, patch)
2010-01-30 11:37 PST, Dirk Schulze
no flags
reimplementation of AffineTransformation (56.35 KB, patch)
2010-01-31 11:27 PST, Dirk Schulze
no flags
reimplementation of AffineTransformation (56.35 KB, patch)
2010-01-31 11:52 PST, Dirk Schulze
no flags
reimplementation of AffineTransformation (57.66 KB, patch)
2010-01-31 23:47 PST, Dirk Schulze
no flags
Add back AffineTransform for SVG (222.28 KB, patch)
2010-02-07 16:59 PST, Dirk Schulze
no flags
Add back AffineTransform for SVG (219.70 KB, patch)
2010-02-08 12:06 PST, Dirk Schulze
no flags
Simon Fraser (smfr)
Comment 1 2010-01-15 16:59:25 PST
Do you have any performance data to show that TransformationMatrix is an issue?
Sam Weinig
Comment 2 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.
Dirk Schulze
Comment 3 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.
Dirk Schulze
Comment 4 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.
WebKit Review Bot
Comment 5 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.
WebKit Review Bot
Comment 6 2010-01-30 12:01:04 PST
Dirk Schulze
Comment 7 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.
WebKit Review Bot
Comment 8 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.
WebKit Review Bot
Comment 9 2010-01-31 11:38:52 PST
Dirk Schulze
Comment 10 2010-01-31 11:52:34 PST
Created attachment 47797 [details] reimplementation of AffineTransformation grrr... typo in GraphicsContextSkia
WebKit Review Bot
Comment 11 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.
Dirk Schulze
Comment 12 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.
Adam Barth
Comment 13 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.
Dirk Schulze
Comment 14 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.
Adam Barth
Comment 15 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. :)
Dirk Schulze
Comment 16 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. :-)
Dirk Schulze
Comment 17 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
Kent Tamura
Comment 18 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
Dirk Schulze
Comment 19 2010-01-31 23:47:27 PST
Created attachment 47815 [details] reimplementation of AffineTransformation fixes windows build.
WebKit Review Bot
Comment 20 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.
Dirk Schulze
Comment 21 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 .
Eric Seidel (no email)
Comment 22 2010-02-01 16:12:02 PST
Attachment 47815 [details] was posted by a committer and has review+, assigning to Dirk Schulze for commit.
Dirk Schulze
Comment 23 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.
Dirk Schulze
Comment 24 2010-02-07 17:02:01 PST
Comment on attachment 47815 [details] reimplementation of AffineTransformation patch landed in r54137.
WebKit Review Bot
Comment 25 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.
WebKit Review Bot
Comment 26 2010-02-07 17:45:24 PST
Adam Barth
Comment 27 2010-02-07 18:23:02 PST
Sorry about the botspam. I think the bots were having some trouble.
Dirk Schulze
Comment 28 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.
Dirk Schulze
Comment 29 2010-02-08 12:06:55 PST
Created attachment 48349 [details] Add back AffineTransform for SVG update was needed after Nikos DRT patch.
WebKit Review Bot
Comment 30 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.
Dirk Schulze
Comment 31 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
Nikolas Zimmermann
Comment 32 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.
Dirk Schulze
Comment 33 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
Note You need to log in before you can comment on or make changes to this bug.