API REVIEW REQUEST: Public API for Node Orientation (original) (raw)

steve.x.northover at oracle.com steve.x.northover at oracle.com
Fri Nov 2 09:03:06 PDT 2012

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

More information about the openjfx-dev mailing list