API REVIEW REQUEST: Public API for Node Orientation

[steve.x.northover at oracle.com]

Hi Pavel,

In order to make more progress on the implementation, we have decided to 
enter JIRA issues for your API suggestions and follow up there.  Leif 
will be entering the JIRA and posting the bug id's back to the list so 
that anyone who is interested can follow along.

Thanks for your input,
Steve

On 30/10/2012 6:41 AM, Pavel Safrata wrote:
> Hi Steve,
>
> On 29.10.2012 16:17, steve.x.northover at oracle.com wrote:
>>
>>
>> On 29/10/2012 7:49 AM, Pavel Safrata wrote:
>>> Hi Steve,
>>>
>>> On 26.10.2012 18:41, steve.x.northover at oracle.com wrote:
>>>>  Hi Pavel!
>>>>
>>>> > the inheritance ignoring reparenting.
>>>>
>>>> I don't think this was explained well in the documentation. There 
>>>> should be no difference in visual behavior for the final result 
>>>> with respect to the ordering of "orientate" and "insert" operations.
>>>
>>> It seems to be explained well. This is how I understand it, please 
>>> tell me which of the two statements is incorrect and why:
>>>
>>> I have a left-to-right parent with an inheriting child. I create new 
>>> parent, "orientate" it to right-to-left and "insert" it between the 
>>> original parent and the child. Based on "If an application 
>>> explicitly sets the root of a hierarchy to left-to-right and then 
>>> reparents the hierarchy into a parent that is right-to-left, the 
>>> hierarchy will remain left-to-right" I understand that the child 
>>> will remain left-to-right.
>>>
>>> Again, I have a left-to-right parent with an inheriting child. I 
>>> create a new parent and "insert" it between the original parent and 
>>> the child. Then I "orientate" it to right-to-left. Based on 
>>> "Inheritance of node orientation allows application developers to 
>>> specify the orientation of a root node and have it apply to all 
>>> children" I understand that the new orientation will be applied to 
>>> the child, so it will become right-to-left.
>>>
>>
>> The second statement is true.  The behavior can be summarized as: 
>> "When not explicitly set, orientation is inherited".  I'm not sure 
>> about the confusion in the first statement.  The sentence is meant to 
>> mean that a hierarchy of nodes with an explicitly set root will 
>> always have the explicitly set orientation of the root no matter 
>> where the root is reparented.  Perhaps I should delete the sentence 
>> and replace it with something like what I just said.
>
> Got it. The confusion is that you mean reparenting the hierarchy 
> including the root, I thought you meant leaving the root on place and 
> reparenting its children to a different root. So I think it is ok (but 
> yes, rewording the sentence may be useful, I'm not the only one to 
> understand it that way).
>
>>
>>>>
>>>> > How will mirroring cooperate with transformations?
>>>>
>>>> The mirroring transformation is transparent to the application and 
>>>> is included automatically in local-to-scene (it's a bug if it is 
>>>> not).  A public Mirror (or rather Flip) transformation would 
>>>> provide API for this transformation, but I'm not sure why we would 
>>>> need to do this.
>>>
>>> Ah, that sounds quite good. The only thing that slightly bothers me 
>>> is the state where there are no transformations anywhere and 
>>> local-to-scene transform still reports it is not an identity 
>>> transform, which seems confusing. But perhaps I'm too picky.
>>>
>>>>
>>>> > Shouldn't effectiveNodeOrientation be a property?
>>>>
>>>> That's a possibility.  It would be a properly that changed when 
>>>> inherited orientation up the ancestor tree changed.  Do we have any 
>>>> other properties like this in FX?
>>>
>>> localToSceneTransform :-) But I admit there is some extra logic 
>>> needed for such properties that we don't want to add blindly for 
>>> performance reasons. So it may be better to just rename the getter 
>>> to simply effectiveNodeOrientation().
>>>
>>
>> It might be that this needs to be a property after all.  The issue is 
>> that a child may have state that is sets based on effective 
>> orientation (say alignment of a text node) and this state needs to be 
>> kept up to date with effective orientation.  However, providing the 
>> method is defined correctly, there is nothing stopping it from 
>> becoming a property in future.  I understand the performance issue.  
>> I will investigate further.
>
> For a property we'd have effectiveNodeOrientationProperty() and 
> getEffectiveNodeOrientation(). For a non-property we'd have something 
> like effectiveNodeOrientation(). So I think we need to decide in the 
> beginning..
>
>>
>>>>
>>>> > The same applies to isAutomaticallyMirrored.
>>>>
>>>> This is a mechanism that allows controls to opt out of mirroring. 
>>>> Conceptually, it should be "... set once in the constructor and 
>>>> never changed...".  I am not particularly happy with this method. 
>>>> Do you have a better suggestion?
>>>
>>> I've just discussed it locally, there are other options but not 
>>> particularly nice as well. Guys here also prefer your solution 
>>> because there is no need to store the value. So I'm withdrawing my 
>>> objections, however, we believe that the method
>>> - needs a better documentation that will state explicitly that it's 
>>> supposed to return a constant
>>> - should be protected (is there any reason for it to be public?)
>>> - needs a name that doesn't start with "get" or "is"
>>
>> I will update the documentation to be better.  Can you show me other 
>> examples where the "get" and "is" are not used in FX where they might 
>> normally be used? 
>
> For instance Point2D.magnitude() or Transform.determinant().
>
> This is for the compatibility with tools and IDEs that use the naming 
> to determine if it is a property or not.
>
>> I am not a fan of protected.  Other than indicating explicitly that 
>> subclasses are supposed to override this method, are there any other 
>> benefits?
>
> I believe it is a good approach not to publish things that don't need 
> to be public. It is a node implementation thing, it should not confuse 
> users in the list of publicly accessible methods. Other than you not 
> being a fan, are there any concrete reasons for it to be public? (the 
> fan thing doesn't leave much room for discussion)
>
>>
>>>
>>>>
>>>> > Could you please elaborate on "the application will need to 
>>>> configure parameters that are appropriate for the effect in both 
>>>> orientations"?
>>>>
>>>> For example, if you want a light source effect to come from the 
>>>> upper left corner when a control is RTL, you will need to create an 
>>>> effect where the light source comes from the upper right corner so 
>>>> that when the control is mirrored, it will come from the left.
>>>
>>> Hmm, I would prefer to do that automatically, I don't think anybody 
>>> wants the reversed shadow just because the reading direction is 
>>> different. But it looks like it would require serious rework of 
>>> effects which is probably not feasible..
>>
>> This issue is this:  You can't know what the application wants. In 
>> some cases, it is using an effect as part of a control theme and it 
>> makes sense for the effect to go from right-to-left when the 
>> orientation changes.  In other cases, there is directionality 
>> involved that should remain constant (like the car example in the 
>> documentation).
>
> I think that effects are quite independent of what the application 
> wants. The reflection always has to reflect the rendered node (having 
> a right-to-left node with left-to-right reflection doesn't make any 
> sense), and I think shadow is always dropped the same way based on the 
> light source, regardless of it being right-to-left text or a car door. 
> But again, I don't see any reasonable way to implement this right now.
>
> Pavel
>
>>
>>>
>>> Pavel
>>>
>>>>
>>>> Steve
>>>>
>>>> On 26/10/2012 9:16 AM, Pavel Safrata wrote:
>>>>> Hi Steve,
>>>>> I have a few comments/questions.
>>>>>
>>>>> I'm not sure about the inheritance ignoring reparenting. I think 
>>>>> that if an application will use orientation extensively it will 
>>>>> reach a hard-to-trace "mess state" where most of the nodes 
>>>>> "inherit" but they don't actually have the parent's value. Also it 
>>>>> means that peforming "orientate parent" - "insert it into scene" 
>>>>> will result in a different behavior than "insert" first and then 
>>>>> "orientate", which seems confusing. What if I create a new node 
>>>>> and insert it into scene, will it inherit form its new parent? In 
>>>>> summary, I find this behavior hard to track and I think that when 
>>>>> the value is Inherit it should always match the parent's orientation.
>>>>>
>>>>> How will mirroring cooperate with transformations? For instance 
>>>>> user can obtain local-to-scene transformation and if the 
>>>>> mirrorring is not contained there, the computations with the 
>>>>> transform (such as transforming points) will be wrong. Maybe we 
>>>>> could just introduce a public Mirror (or rather Flip) 
>>>>> transformation and use it publicly for the mirrorring?
>>>>>
>>>>> How will it behave in 3D? Mirror nodes along X axis regardless of 
>>>>> their z-direction volume?
>>>>>
>>>>> Shouldn't effectiveNodeOrientation be a property? It seems it 
>>>>> might make sense to observe the value. Also our naming convention 
>>>>> is that you should not use getSomthing unless "something" is a 
>>>>> property.
>>>>>
>>>>> The same applies to isAutomaticallyMirrored. This method seems 
>>>>> weird anyway. When and how often is it called? Can a node change 
>>>>> the value dynamically? If yes, we should have a property, if not, 
>>>>> we should make sure it doesn't - let the node call some init 
>>>>> method in the constructor or something like that.
>>>>>
>>>>> Could you please elaborate on "the application will need to 
>>>>> configure parameters that are appropriate for the effect in both 
>>>>> orientations"? How do I drop the shadow to the same direction for 
>>>>> all nodes, specifically?
>>>>>
>>>>> Thanks,
>>>>> Pavel
>>>>>
>>>>> On 23.10.2012 22:30, steve.x.northover at oracle.com wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> I have been looking into Node Orientation which is an API that 
>>>>>> controls the directionality of a Node.  This is different from 
>>>>>> BIDI or the BIDI algorithm which governs the direction of text. 
>>>>>> Node orientation concerns the flow of visual data which is either 
>>>>>> left-to-right or right-to-left.  The best example is a tree 
>>>>>> control.  In tree control that is oriented right-to-left, the 
>>>>>> expansion arrows point to the right and the branches of the tree 
>>>>>> expand from the right to the left.
>>>>>>
>>>>>> https://wikis.oracle.com/display/OpenJDK/Node+Orientation+in+JavaFX
>>>>>>
>>>>>> Steve
>>>>>
>>>
>