[Webkit-unassigned] [Bug 113318] [GTK][AC] Support preserves3D css property for clutter ac backend.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=113318


Gustavo Noronha (kov) <gns at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #195839|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #6 from Gustavo Noronha (kov) <gns at gnome.org>  2013-04-01 10:00:00 PST ---
(From update of attachment 195839)
View in context: https://bugs.webkit.org/attachment.cgi?id=195839&action=review

r- because of the comments

> Source/WebCore/ChangeLog:9
> +        Most of all codes are based on Mac port's implementation. The core concept is that

s/Most of all codes are/Most of the code is/

> Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:48
> +    gboolean flattening;

flatten is a better name since it's imperative, or flattens to follow the drawContents convention, present continuous indicates things are happening at that time, usually we use that only for tracking whether something is happening - like stating that layout is underway to avoid triggering it again; btw, we can use bool here instead of gboolean, I think we should!

> Source/WebCore/platform/graphics/clutter/GraphicsLayerActor.cpp:517
> +void graphicsLayerActorSetFlattening(GraphicsLayerActor* layer, gboolean flattening)

We should change the method name too.

> Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:584
>      // FIXME: Save the state before sending down to kids and restore it after
>      TransformState localState = state;
> +    CommitState childCommitState = commitState;

Looks like the state is not restored anywhere. Is this what the FIXME is about? Is it because the child tries to change the state that is passed to it, so we need a modifiable copy of those? Could we drop the assignments for now and add them when we are actually restoring state?

> Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:712
> +        // If the preserves3D of a layer is false, itself and its immediate children should be flattened.
> +        graphicsLayerActorSetFlattening(childLayer, !(preserves3D() || currentChild->preserves3D()));

Not sure I understand this comment. What this is doing is telling the child to flatten if neither the "this" layer nor the currentChild has preserve3D set. If that is correct behaviour, then I think we can do away with the comment. The comment implies the layer and its children should be flattened if the "this" layer has preserves3D set to false.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.