WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
73767
[GTK] Implement GraphicsLayer using Clutter
https://bugs.webkit.org/show_bug.cgi?id=73767
Summary
[GTK] Implement GraphicsLayer using Clutter
Joone Hur
Reported
2011-12-04 04:21:12 PST
This patch allows us to run CSS3 3D transforms in WebKitGtk+ when Accelerated Compositing is enabled.
Attachments
Initial patch (no change log)
(60.31 KB, patch)
2012-06-28 08:56 PDT
,
Joone Hur
no flags
Details
Formatted Diff
Diff
test case (rotate3d)
(413 bytes, text/html)
2012-06-28 08:57 PDT
,
Joone Hur
no flags
Details
Patch
(15.72 KB, patch)
2012-08-19 19:38 PDT
,
Joone Hur
no flags
Details
Formatted Diff
Diff
Extra jhbuild modulesets for building Clutter and its dependencies
(6.22 KB, application/octet-stream)
2012-08-20 01:29 PDT
,
Joone Hur
no flags
Details
Patch2
(15.36 KB, patch)
2012-08-20 02:17 PDT
,
Joone Hur
no flags
Details
Formatted Diff
Diff
Patch
(21.04 KB, patch)
2012-09-21 14:46 PDT
,
Joone Hur
no flags
Details
Formatted Diff
Diff
Martin & Carlos, thanks for the review
(16.03 KB, patch)
2012-09-23 09:39 PDT
,
Joone Hur
no flags
Details
Formatted Diff
Diff
Patch
(15.61 KB, patch)
2012-09-24 09:56 PDT
,
Joone Hur
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Alejandro G. Castro
Comment 1
2012-06-15 04:29:04 PDT
What is your plan about this? I thin it is already working?
Joone Hur
Comment 2
2012-06-17 00:54:10 PDT
(In reply to
comment #1
)
> What is your plan about this? I thin it is already working?
Yes, I have a working repository:
https://gitorious.org/~joone/webkit/joone-webkit-ac
But, it doesn't work with trunk so I tried to make it work with trunk. Currently, I was able to build clutter-ac with trunk, but it has some issues to run. I am fixing it now. Please wait a bit more and I will upload the full patch first or make another working repository. Thank you!
Joone Hur
Comment 3
2012-06-28 08:56:12 PDT
Created
attachment 149960
[details]
Initial patch (no change log) I tried to make Clutter backend patch as small as possible, so it only supports a few examples like rotate3d.
Joone Hur
Comment 4
2012-06-28 08:57:59 PDT
Created
attachment 149961
[details]
test case (rotate3d)
Martin Robinson
Comment 5
2012-06-28 09:02:49 PDT
Comment on
attachment 149960
[details]
Initial patch (no change log) A couple things from a quick skim of the patch: 1. GObject source code should follow WebKit coding style. You should use webkitNamingConventions for methods and spacing. 2. Perhaps it's better to add the skeleton to make this compile and gradually implement things. 3 You shouldn't need to add clutter dependencies to the GTK+ jhbuild. The bots aren't building this now and they aren't necessary to make tests pass.
WebKit Review Bot
Comment 6
2012-06-28 09:12:10 PDT
Attachment 149960
[details]
did not pass style-queue: Source/WebCore/platform/graphics/clutter/webkitclutterinclude.h:19: #ifndef header guard has wrong style, please use: webkitclutterinclude_h [build/header_guard] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.h:32: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.h:122: The parameter name "flags" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/gtk/webkit/webkitglobals.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:19: #ifndef header guard has wrong style, please use: GraphicsLayerActor_h [build/header_guard] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:22: Header file should not contain WebCore config.h. Should be: alphabetically sorted. [build/include_order] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:27: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:36: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:36: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:37: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:40: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:40: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:41: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:44: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:44: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:45: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:48: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:52: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:52: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:53: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:66: This { should be at the end of the previous line [whitespace/braces] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:67: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:69: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:70: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:74: This { should be at the end of the previous line [whitespace/braces] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:75: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:75: parent_class is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:77: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:78: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:78: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:79: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:79: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:80: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:80: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:81: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:81: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:82: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:82: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:85: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:85: graphics_layer_actor_get_type is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:87: graphics_layer_actor_ is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:88: graphics_layer_actor__with_client is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:89: graphics_layer_actor_set_client is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:90: graphics_layer_actor_get_client is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:91: graphics_layer_actor_remove_all is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:92: graphics_layer_actor_get_surface is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:93: graphics_layer_actor_set_surface is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:94: graphics_layer_actor_invalidate_rectangle is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:95: graphics_layer_actor_set_transform is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:96: graphics_layer_actor_set_anchor_point is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:97: graphics_layer_actor_get_anchor_point is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:98: graphics_layer_actor_set_scroll_position is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:99: graphics_layer_actor_set_translate_x is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:100: graphics_layer_actor_get_translate_x is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:101: graphics_layer_actor_set_translate_y is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:102: graphics_layer_actor_get_translate_y is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:103: graphics_layer_actor_get_n_children is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:104: graphics_layer_actor_get_layer_type is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:105: graphics_layer_actor_set_layer_type is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:106: graphics_layer_actor_set_sublayers is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:107: graphics_layer_actor_get_draws_content is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.h:108: graphics_layer_actor_set_draws_content is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.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] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:25: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:28: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:36: The parameter name "iface" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:36: clutter_container_iface_init is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:37: container_add is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:37: The parameter name "container" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:38: The parameter name "actor" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:39: container_remove is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:39: The parameter name "container" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:40: The parameter name "actor" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:49: This { should be at the end of the previous line [whitespace/braces] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:62: anchor_x is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:63: anchor_y is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:64: anchor_z is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:66: scroll_x is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:67: scroll_y is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:69: translate_x is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:70: translate_y is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:75: This { should be at the end of the previous line [whitespace/braces] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:85: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:86: Extra space between guint and prop_id [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:86: prop_id is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:88: Declaration has space between type name and * in GParamSpec *pspec [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:101: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:106: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:107: Extra space between guint and prop_id [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:107: prop_id is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:108: Extra space between GValue and *value [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:108: Declaration has space between type name and * in GValue *value [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:109: Declaration has space between type name and * in GParamSpec *pspec [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:121: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:122: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:128: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:143: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:150: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:176: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:181: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:209: graphics_layer_actor_apply_transform is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:224: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:231: pivot_x is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:231: pivot_y is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:264: graphics_layer_actor_draw is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:270: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:279: surface_width is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:280: surface_height is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:293: graphics_layer_actor_paint is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:357: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:359: Declaration has space between type name and * in GObjectClass *object_class [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:359: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:360: Declaration has space between type name and * in ClutterActorClass *actor_class [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:360: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:361: Declaration has space between type name and * in GParamSpec *pspec [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:373: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:406: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:424: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:430: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:468: container_add is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:486: container_remove is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:500: container_replace is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:517: container_foreach is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:519: user_data is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:525: This { should be at the end of the previous line [whitespace/braces] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:531: clutter_container_iface_init is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:541: graphics_layer_actor_remove_all is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:543: Declaration has space between type name and * in GList *children [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:548: Declaration has space between type name and * in ClutterActor *child [whitespace/declaration] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:583: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:590: graphics_layer_actor_get_surface is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:599: graphics_layer_actor_set_surface is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:606: graphics_layer_actor_invalidate_rectangle is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:617: graphics_layer_actor_set_transform is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:642: graphics_layer_actor_set_anchor_point is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:657: graphics_layer_actor_get_anchor_point is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:670: graphics_layer_actor_set_scroll_position is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:681: graphics_layer_actor_get_n_children is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:685: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:688: graphics_layer_actor_replace_sublayer is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:697: graphics_layer_actor_insert_sublayer is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:712: graphics_layer_actor_set_sublayers is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:714: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:727: graphics_layer_actor_get_layer_type is incorrectly named. Don't use underscores in your identifFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/GNUmakefile.list.am', u'Sou..." exit_code: 1 ier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:733: graphics_layer_actor_set_layer_type is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:739: graphics_layer_actor_set_translate_x is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:746: graphics_layer_actor_get_translate_x is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:752: graphics_layer_actor_set_translate_y is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:759: graphics_layer_actor_get_translate_y is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:766: graphics_layer_actor_set_draws_content is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:778: graphics_layer_actor_get_draws_content is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 153 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Gustavo Noronha (kov)
Comment 7
2012-06-28 09:30:23 PDT
Comment on
attachment 149960
[details]
Initial patch (no change log) View in context:
https://bugs.webkit.org/attachment.cgi?id=149960&action=review
I think I would like to have this split quite a bit. We should also fix all of the style issues, of course, I'd recommend running the style checker, doing the fixes and pushing them to the webkit-clutter repository first, so we don't desync. Also important to have all of the latest fixes from there. I think we can split this into one patch adding GraphicsLayerActor (with all of the style fixes), one adding TransformMatrixClutter, one with GraphicsLayerClutter.
> Source/WebCore/platform/clutter/GRefPtrClutter.cpp:23 > -#include <clutter/clutter.h> > +#include "webkitclutterinclude.h"
Let's not do this. It's required for a specific version of clutter/cogl, but we should avoid having it for trunk, just depend on a new enough clutter and remove webkitclutterinclude.
Joone Hur
Comment 8
2012-07-01 02:20:43 PDT
(In reply to
comment #7
)
> (From update of
attachment 149960
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=149960&action=review
> > I think I would like to have this split quite a bit. We should also fix all of the style issues, of course, I'd recommend running the style checker, doing the fixes and pushing them to the webkit-clutter repository first, so we don't desync. Also important to have all of the latest fixes from there. I think we can split this into one patch adding GraphicsLayerActor (with all of the style fixes), one adding TransformMatrixClutter, one with GraphicsLayerClutter. > > > Source/WebCore/platform/clutter/GRefPtrClutter.cpp:23 > > -#include <clutter/clutter.h> > > +#include "webkitclutterinclude.h" > > Let's not do this. It's required for a specific version of clutter/cogl, but we should avoid having it for trunk, just depend on a new enough clutter and remove webkitclutterinclude.
Oops! I should not have run EWS. Actually, I didn't fix all of the style issues. I just wanted to check if the size of this patch could be manageable. Okay, I will split this patch into separate pieces for each file, and apply the style fixes to webkit-clutter first, and not include "webkitclutterinclude.h" for trunk.
> 3 You shouldn't need to add clutter dependencies to the GTK+ jhbuild. The bots aren't building this now and they aren't necessary to make tests pass.
@Martin: Yes, Clutter is not needed for the default build options, but it makes it easier to build WebKitGtk+ with a specific version of Clutter. It would be good that we can add specific components to jhbuild according to build options. Thanks for the comments!
Martin Robinson
Comment 9
2012-07-01 16:21:56 PDT
(In reply to
comment #8
)
> @Martin: Yes, Clutter is not needed for the default build options, but it makes it easier to build WebKitGtk+ with a specific version of Clutter. It would be good that we can add specific components to jhbuild according to build options.
The jhbuild is not really for satisfying dependencies, but for making it so that tests pass on all platforms. Perhaps we should use it for satisfying dependencies in the future, but not sure if we should change that in this patch.
Philippe Normand
Comment 10
2012-07-01 19:03:43 PDT
(In reply to
comment #9
)
> (In reply to
comment #8
) > > > @Martin: Yes, Clutter is not needed for the default build options, but it makes it easier to build WebKitGtk+ with a specific version of Clutter. It would be good that we can add specific components to jhbuild according to build options. > > The jhbuild is not really for satisfying dependencies, but for making it so that tests pass on all platforms. Perhaps we should use it for satisfying dependencies in the future, but not sure if we should change that in this patch.
See also
https://trac.webkit.org/wiki/BuildingGtk#Extendingthejhbuildenvironment
Joone Hur
Comment 11
2012-07-23 08:32:07 PDT
(In reply to
comment #10
)
> (In reply to
comment #9
) > > (In reply to
comment #8
) > > > > > @Martin: Yes, Clutter is not needed for the default build options, but it makes it easier to build WebKitGtk+ with a specific version of Clutter. It would be good that we can add specific components to jhbuild according to build options. > > > > The jhbuild is not really for satisfying dependencies, but for making it so that tests pass on all platforms. Perhaps we should use it for satisfying dependencies in the future, but not sure if we should change that in this patch. > > See also >
https://trac.webkit.org/wiki/BuildingGtk#Extendingthejhbuildenvironment
@Philippe: Thanks for the information. It was very helpful. :-) @Kov: I've made a separated patch for GraphicsLayerActor.
https://bugs.webkit.org/show_bug.cgi?id=91940
Joone Hur
Comment 12
2012-08-19 19:38:03 PDT
Created
attachment 159320
[details]
Patch
Joone Hur
Comment 13
2012-08-20 01:29:44 PDT
Created
attachment 159362
[details]
Extra jhbuild modulesets for building Clutter and its dependencies You can build WebKitGtk+ with Clutter and its dependencies by setting the environment variables as follows: export WEBKIT_EXTRA_MODULESETS=file:///home/joone/git/WebKit/Tools/gtk/jhbuild_clutter.modules export WEBKIT_EXTRA_MODULES=webkitgtk-clutter-dependencies
Joone Hur
Comment 14
2012-08-20 02:17:51 PDT
Created
attachment 159374
[details]
Patch2
WebKit Review Bot
Comment 15
2012-08-20 02:21:12 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See
http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Martin Robinson
Comment 16
2012-08-20 15:28:06 PDT
Comment on
attachment 159374
[details]
Patch2 View in context:
https://bugs.webkit.org/attachment.cgi?id=159374&action=review
> Source/WebCore/platform/clutter/GRefPtrClutter.cpp:66 > +template <> GRefPtr<GraphicsLayerActor> adoptGRef(GraphicsLayerActor* ptr) > +{ > + if (g_object_is_floating(ptr)) > + g_object_ref_sink(ptr); > + > + return GRefPtr<GraphicsLayerActor>(ptr, GRefPtrAdopt); > +} > + > +template <> GraphicsLayerActor* refGPtr<GraphicsLayerActor>(GraphicsLayerActor* ptr) > +{ > + if (ptr) { > + if (g_object_is_floating(ptr)) > + g_object_ref_sink(ptr); > +
Isn't this the same as the default GRefPtr implementation? If that's the case, you can just remove this file completely.
Joone Hur
Comment 17
2012-09-16 09:32:52 PDT
Comment on
attachment 159374
[details]
Patch2 View in context:
https://bugs.webkit.org/attachment.cgi?id=159374&action=review
>> Source/WebCore/platform/clutter/GRefPtrClutter.cpp:66 >> + > > Isn't this the same as the default GRefPtr implementation? If that's the case, you can just remove this file completely.
I tried to remove GRefPtrClutter, but I got a runtime error: (GtkLauncher:28198): GLib-GObject-WARNING **: invalid uninstantiatable type `<invalid>' in cast to `ClutterActor' (GtkLauncher:28198): Clutter-CRITICAL **: clutter_container_add_actor: assertion `CLUTTER_IS_ACTOR (actor)' failed ** ERROR:../../Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:401:void WebCore::GraphicsLayerClutter::updateSublayerList(): assertion failed: (GRAPHICS_LAYER_IS_ACTOR(childLayer)) I think GRefPtrClutter needs to be added.
Martin Robinson
Comment 18
2012-09-16 09:39:40 PDT
(In reply to
comment #17
)
> (From update of
attachment 159374
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=159374&action=review
> > >> Source/WebCore/platform/clutter/GRefPtrClutter.cpp:66 > >> + > > > > Isn't this the same as the default GRefPtr implementation? If that's the case, you can just remove this file completely. > > I tried to remove GRefPtrClutter, but I got a runtime error: > > (GtkLauncher:28198): GLib-GObject-WARNING **: invalid uninstantiatable type `<invalid>' in cast to `ClutterActor' > > (GtkLauncher:28198): Clutter-CRITICAL **: clutter_container_add_actor: assertion `CLUTTER_IS_ACTOR (actor)' failed > ** > ERROR:../../Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:401:void WebCore::GraphicsLayerClutter::updateSublayerList(): assertion failed: (GRAPHICS_LAYER_IS_ACTOR(childLayer)) > > I think GRefPtrClutter needs to be added.
Do you know what code is causing these assertion failures? Isn't the only difference between GRefPtrClutter and the vanilla GRefPtr is the type that it's cast to?
Joone Hur
Comment 19
2012-09-17 09:47:28 PDT
Comment on
attachment 159374
[details]
Patch2 View in context:
https://bugs.webkit.org/attachment.cgi?id=159374&action=review
>>>> Source/WebCore/platform/clutter/GRefPtrClutter.cpp:66 >>>> + >>> >>> Isn't this the same as the default GRefPtr implementation? If that's the case, you can just remove this file completely. >> >> I tried to remove GRefPtrClutter, but I got a runtime error: >> >> (GtkLauncher:28198): GLib-GObject-WARNING **: invalid uninstantiatable type `<invalid>' in cast to `ClutterActor' >> >> (GtkLauncher:28198): Clutter-CRITICAL **: clutter_container_add_actor: assertion `CLUTTER_IS_ACTOR (actor)' failed >> ** >> ERROR:../../Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:401:void WebCore::GraphicsLayerClutter::updateSublayerList(): assertion failed: (GRAPHICS_LAYER_IS_ACTOR(childLayer)) >> >> I think GRefPtrClutter needs to be added. > > Do you know what code is causing these assertion failures? Isn't the only difference between GRefPtrClutter and the vanilla GRefPtr is the type that it's cast to?
refGPtr is different between GRefPtrClutter and the vanilla GRefPtr, so I replaced the refGPtr of the vanilla GRefPtr with the refGPtr method of GRefPtrClutter, then AC started working.
Carlos Garcia Campos
Comment 20
2012-09-18 00:32:32 PDT
Comment on
attachment 159374
[details]
Patch2 View in context:
https://bugs.webkit.org/attachment.cgi?id=159374&action=review
>>>>> Source/WebCore/platform/clutter/GRefPtrClutter.cpp:66 >>>>> + >>>> >>>> Isn't this the same as the default GRefPtr implementation? If that's the case, you can just remove this file completely. >>> >>> I tried to remove GRefPtrClutter, but I got a runtime error: >>> >>> (GtkLauncher:28198): GLib-GObject-WARNING **: invalid uninstantiatable type `<invalid>' in cast to `ClutterActor' >>> >>> (GtkLauncher:28198): Clutter-CRITICAL **: clutter_container_add_actor: assertion `CLUTTER_IS_ACTOR (actor)' failed >>> ** >>> ERROR:../../Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:401:void WebCore::GraphicsLayerClutter::updateSublayerList(): assertion failed: (GRAPHICS_LAYER_IS_ACTOR(childLayer)) >>> >>> I think GRefPtrClutter needs to be added. >> >> Do you know what code is causing these assertion failures? Isn't the only difference between GRefPtrClutter and the vanilla GRefPtr is the type that it's cast to? > > refGPtr is different between GRefPtrClutter and the vanilla GRefPtr, so I replaced the refGPtr of the vanilla GRefPtr with the refGPtr method of GRefPtrClutter, then AC started working.
The implementation is indeed different. This adoptGRef consumes the floating reference in case the object has a floating ref, which I'm not sure it's a good idea, because adoptGRef shouldn't change the references at all, IMO. And refGPtr increases the reference counter twice in case the object has a floating reference, once to consume the floating reference and again to increase the reference counter to 2. I don't think this is the expected behaviour of GRefPtr, so maybe this should be a new RefPtr object GFloatingRef or something like that. I don't know why you need this, though.
Martin Robinson
Comment 21
2012-09-18 08:09:36 PDT
(In reply to
comment #20
)
> (From update of
attachment 159374
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=159374&action=review
> The implementation is indeed different. This adoptGRef consumes the floating reference in case the object has a floating ref, which I'm not sure it's a good idea, because adoptGRef shouldn't change the references at all, IMO. And refGPtr increases the reference counter twice in case the object has a floating reference, once to consume the floating reference and again to increase the reference counter to 2. I don't think this is the expected behaviour of GRefPtr, so maybe this should be a new RefPtr object GFloatingRef or something like that. I don't know why you need this, though.
refGPtr sinks floating references, so if your GObject has a floating reference, consider it unreffed and put it in the smart pointer like this: GRefPtr<ClutterThing> smartPointer = clutterThing;
Joone Hur
Comment 22
2012-09-21 14:46:42 PDT
Created
attachment 165193
[details]
Patch
Martin Robinson
Comment 23
2012-09-21 14:52:32 PDT
Comment on
attachment 165193
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=165193&action=review
Thanks for this iteration! Looks good, but just a few minor comments.
> Source/WebCore/platform/graphics/clutter/TransformationMatrixClutter.cpp:34 > + matrix.xx = (float)m11(); > + matrix.xy = (float)m21(); > + matrix.xz = (float)m31(); > + matrix.xw = (float)m41();
Why are these casts here? Can't you convert a double into a float without a cast?
> Tools/MiniBrowser/gtk/GNUmakefile.am:14 > + $(CLUTTER_CFLAGS) \
Instead of adding these CFLAGS and LIBS to all of the programs that use the WebKit library, you should add them to the library itself. WebKit1 and WebKit2 should be sufficient. Since the programs don't use the libraries directly, you shouldn't need to add them. By not linking libwebkit* against clutter, you'll force anyone using libwebkit to add libclutter to their linker line.
Joone Hur
Comment 24
2012-09-22 10:19:35 PDT
Comment on
attachment 165193
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=165193&action=review
>> Source/WebCore/platform/graphics/clutter/TransformationMatrixClutter.cpp:34 >> + matrix.xw = (float)m41(); > > Why are these casts here? Can't you convert a double into a float without a cast?
Do you mean static_cast should be used here?
>> Tools/MiniBrowser/gtk/GNUmakefile.am:14 >> + $(CLUTTER_CFLAGS) \ > > Instead of adding these CFLAGS and LIBS to all of the programs that use the WebKit library, you should add them to the library itself. WebKit1 and WebKit2 should be sufficient. Since the programs don't use the libraries directly, you shouldn't need to add them. > > By not linking libwebkit* against clutter, you'll force anyone using libwebkit to add libclutter to their linker line.
Okay, I will remove them.
Martin Robinson
Comment 25
2012-09-22 10:31:01 PDT
Comment on
attachment 165193
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=165193&action=review
>>> Source/WebCore/platform/graphics/clutter/TransformationMatrixClutter.cpp:34 >>> + matrix.xw = (float)m41(); >> >> Why are these casts here? Can't you convert a double into a float without a cast? > > Do you mean static_cast should be used here?
It seems like you could remove the cast entirely. If not, just use static_cast.
Joone Hur
Comment 26
2012-09-23 09:39:18 PDT
Created
attachment 165285
[details]
Martin & Carlos, thanks for the review
Martin Robinson
Comment 27
2012-09-23 16:58:19 PDT
Comment on
attachment 165285
[details]
Martin & Carlos, thanks for the review View in context:
https://bugs.webkit.org/attachment.cgi?id=165285&action=review
> Source/WebCore/platform/graphics/clutter/PlatformClutterLayerClient.h:39 > +#include <glib.h> > +#include <wtf/HashMap.h> > +#include <wtf/PassRefPtr.h> > +#include <wtf/RefCounted.h> > +#include <wtf/Vector.h> > +#include <wtf/text/StringHash.h>
It seems some of these aren't necessary. Maybe you could remove them before landing?
> Source/WebCore/platform/graphics/clutter/TransformationMatrixClutter.cpp:34 > + matrix.xx = static_cast<float>(m11()); > + matrix.xy = static_cast<float>(m21()); > + matrix.xz = static_cast<float>(m31()); > + matrix.xw = static_cast<float>(m41());
I'm really not sure if these explicit casts are necessary... I created this small program on my machine: #include <stdio.h> int main(int argc, char* argv[]) { double foo = 3.4; float bar; bar = foo; printf("bar: %f\n", bar); return 0; } and compiled it with g++ -Wall. I didn't see any warnings. Will you please try to remove them before landing?
Joone Hur
Comment 28
2012-09-24 09:41:23 PDT
Comment on
attachment 165285
[details]
Martin & Carlos, thanks for the review View in context:
https://bugs.webkit.org/attachment.cgi?id=165285&action=review
>> Source/WebCore/platform/graphics/clutter/PlatformClutterLayerClient.h:39 >> +#include <wtf/text/StringHash.h> > > It seems some of these aren't necessary. Maybe you could remove them before landing?
Yes, some of these are unnecessary. I will remove them.
>> Source/WebCore/platform/graphics/clutter/TransformationMatrixClutter.cpp:34 >> + matrix.xw = static_cast<float>(m41()); > > I'm really not sure if these explicit casts are necessary... > > I created this small program on my machine: > #include <stdio.h> > > int main(int argc, char* argv[]) > { > double foo = 3.4; > float bar; > bar = foo; > printf("bar: %f\n", bar); > return 0; > } > > and compiled it with g++ -Wall. I didn't see any warnings. Will you please try to remove them before landing?
There are some cases of using explicit cast to covert double to float in WebKit code, but it's okay to remove these explicit casts if there are no warnings when compiling the code.
Joone Hur
Comment 29
2012-09-24 09:56:22 PDT
Created
attachment 165404
[details]
Patch
WebKit Review Bot
Comment 30
2012-09-24 10:44:55 PDT
Comment on
attachment 165404
[details]
Patch Clearing flags on attachment: 165404 Committed
r129387
: <
http://trac.webkit.org/changeset/129387
>
WebKit Review Bot
Comment 31
2012-09-24 10:45:01 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug