[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





--- Comment #10 from ChangSeok Oh <kevin.cs.oh at gmail.com>  2013-04-01 22:45:18 PST ---
(From update of attachment 195839)
View in context: https://bugs.webkit.org/attachment.cgi?id=195839&action=review

Thank you for the comments!

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

Done.

>> Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:584
>> +    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?

O.K. Make sense. We're not using them, so we don't need to keep them as local variables.

>>> Source/WebCore/platform/graphics/clutter/GraphicsLayerClutter.cpp:712
>>> +        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.
> 
> All elements transform-style property has false value by default, so preserve3D() usually returns false. I think we can change the comment as follws:
> "If the preserve3D() of this layer returns true, its children should not be flattened"
> 
> By the way, do we really need to check currentChild->preserves3D()?

"If the preserve3D() of this layer returns true, itself and its children should not be flattened" is better to me.
But I don't mind removing the comment if it causes that you're confused. :p

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