Bug ID: JDK-8144556 Add support to allow user specified rendering order

JDK 9
9Fixed

Changeset: 9f243fcb6ea1 Author: ckyang Date: 2016-04-18 13:39 -0700 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/9f243fcb6ea1
18-04-2016
+1
18-04-2016
1) Fixed test bug in HelloViewOrder. There is a missing reset to zero in translate (also for the case in viewOrder) after unbinding the wheatRect. 2) Fixed format issue in Node webrev v4: http://cr.openjdk.java.net/~ckyang/JDK-8144556/webrev.04/
16-04-2016
* Bug in HelloViewOrder 1. Run HelloViewOrder 2. Select Translate 3. Unselect translate 4. Select viewOrder 5. Press Add 6. Move the slider -- nothing happens * Additionally there is a very minor formatting issue that I forgot to mention the first time, that you might want to fix as long as you will need a new webrev for the functional issue. Node.java: + /* ************************************************************************* + * * + * viewOrder property handling * + * * + *************************************************************************/ The space after the '/' on the first line should be an '*' to match the formatting of other similar blocks.
15-04-2016
Here is the webrev that includes: 1) Added new unit tests to verify the state and data of parent's viewOrderChildren list 2) Improved HelloViewOrder and removed HelloViewOrderStack 3) Fixed JavaDoc, comments and code as suggested. webrev v3: http://cr.openjdk.java.net/~ckyang/JDK-8144556/webrev.03/
15-04-2016
Here are my review comments on version .02: API: The API looks good and is approved. ------------------- Docs: * Minor corrections for which I sent off-line feedback * The cssref.html docs is a copy of what is in the Node.java file. This will be hard to maintain and isn't what we do for others. An abbreviated version would be better. ------------------- Testing: * A few more unit tests would be very helpful, as discussed off-line * Minor corrections to the HelloViewOrder program (and removal of the HelloViewOrderStack program) as discussed off-line ------------------- Code review: Overall it looks good. My testing shows no issues, either. Here are a collection of mostly minor issues. DirtyBits.java 1. The following line was mistakenly duplicated: // Dirty bits for the Node class +// Dirty bits for the Node class NGGroup.java 2. In the following: + * The viewOrderChildren is a list children sorted in viewOrder if it is not + * null. Its size should always be equal to children.size(). If + * viewOrderChildren is null it implies that the rendering order of the The list is always non-null. I think you mean "empty" rather than "null" in both of the above places, right? + final private List<NGNode> viewOrderChildren = new ArrayList<>(1); The order should be "private final ..." 3. The following method takes a list of FX Nodes and sorts them: public void setViewOrderChildren(List<Node> sortedChildren) The usual pattern would be to do this on the FX side and pass down the list of NGNodes. If you want to keep the existing code, that seems OK, but in that case you should codument that the setViewOrderChildren is called on the FX application thread with the RenderLock held. The method is unsafe otherwise, so that assumption should be documented. 4. Can you fix the spacing (by adding a space before and after the '=' and '>=') in the following line, since you are modifying it: + for (int resultIdx=orderedChildren.size()-1; resultIdx>=0; resultIdx--) { Parent.java 5. Same comment as above for NGGroup sorted children list: + /** + * The viewOrderChildren is a list children sorted in viewOrder if it is + * not null. Its size should always be equal to children.size(). If + * viewOrderChildren is null it implies that the rendering order of the The list is always non-null. + final private List<Node> viewOrderChildren = new ArrayList(1); private final ... 6. Maybe change the following to "vo" since the name is now viewOrder and not priorityOrder? + double po = child.getViewOrder(); 7. The following has a couple typos in the second line: + // Mark viewOrderChildrenDirty if there is modification to children list + // because priorty order was set on one of more children "priorty" (sic) should be "view"; the last part should be "on one or more children"; it might be clearer if "because" were changed to "and" and if you also made it clear that the existing children have view order set. How about this: // and view order was set on one or more of the children prior to this change HelloViewOrder.java 8. Please add a blank line before the package statement: 24 */ 25 package hello; ParentHelper.java 9. Ditto the above comment ViewOrderTest.java 10. The extra blank lines in the middle of methods can be removed: 65 66 122 123
15-04-2016
Here is the webrev that includes: 1) Name change from priorityOrder to viewOrder 2) Cleaner the implementation 3) Added another toys test, HelloViewOrderStack, using simple key press to update viewOrder and add and remove Node. webrev v2: http://cr.openjdk.java.net/~ckyang/JDK-8144556/webrev.02/
08-04-2016
We concluded that priorityOrder can be confusing to some people as to the order of a node is rendered or picked within its parent. Hence we decided to use viewOrder as it can be easier to relate to the distance between the node and the viewer (camera), i.e. smaller is closer. Here is the specification: public final DoubleProperty viewOrderProperty Defines the rendering and picking order of this Node within its parent. This property is used to alter the rendering and picking order of a node within its parent without reordering the parent's children list. For example this can be used as a more efficient way to implement transparency sorting. To do this, an application can assign the viewOrder value of each node to the computed distance between that node and the viewing Camera. The parent will traverse its children in decreasing viewOrder order. This means that a child with a lower viewOrder will be in front of a child with a higher viewOrder. If two children have the same viewOrder, the parent will traverse them in the order they appear in the parent's children list. However, viewOrder does not alter the layout and focus traversal order of this Node within its parent. A parent always traverses its children list in order when doing layout or focus traversal. Default value: 0.0 Since: 9 See Also: getViewOrder(), setViewOrder(double)
08-04-2016
Attached PriorityOrderTest3.java that include various shapes to illustrate another example of using prioriyOrder to address the dirty edges issue as reported in JDK-8094313.
06-04-2016
I'm attaching a simple example to illustrate the beauty of matching priorityOrder with z-ordering. To do proper multilayers transparency rendering an application will need to match the layers' rendering order with their z-order. In this example, I can simply bind the slider's value to wheatRect's translateZ and priorityOrder as I move wheatRect between the other 3 rectangles. wheatRect.translateZProperty().bind(slider.valueProperty()); wheatRect.priorityOrderProperty().bind(slider.valueProperty()); The program is PriorityOrderTest2.java The other question to ask is that what is the main need for priorityOrder. If it is for ordered rendering related to depth then I believe having priorityOrder matching z-order is a good choice. At least in the above example it does illustrate a case for making doing simple thing simple.
04-04-2016
@Jonathan, this was one of my early concerns as well. The only reason for choosing the current ordering was to match the ordering for Z translation and Z values. If we were to think about this API independent of that, I would have recommended the ordering that you (and perhaps most people) would find more natural, with higher priority nodes drawn on top of lower priority nodes. I also would have echoed your earlier suggestion of making it an int property. So the question is: is there enough value in having it match the Z order for the (common) use case of directly putting a value computed from Z into that field to justify the somewhat less intuitive ordering? Given that transforms don't affect the priority order, I can easily imagine that it might not even be that straight-forward to directly use Z anyway. We can discuss this further next week.
02-04-2016
This is spoken by a non-graphics person, but to me this seems around the wrong way. Wouldn't most people assume (like I did) that a higher priority order will result in the node being 'higher' in the stack, i.e. towards the front, rather than further away and behind the lower priority nodes? Or maybe I'm totally misunderstanding given your statement that 'priorityOrder does not alter the layout'.
01-04-2016
Thanks! I have fixed the typo in cssref doc and thanks to Kevin for helping to reword the JavaDoc. I hope this is a better and concise specification. The only change in this webrev are cssref and java doc fixes: http://cr.openjdk.java.net/~ckyang/JDK-8144556/webrev.01/ public final DoubleProperty priorityOrderProperty Defines the rendering and picking order of this Node within its parent. This property is used to alter the rendering and picking order of a node within its parent without reordering the parent's children list. For example this can be used as a more efficient way to implement transparency sorting. The parent will traverse its children in decreasing priorityOrder order. This means that a child with a lower priorityOrder will be in front of a child with a higher priorityOrder. If two children have the same priorityOrder, the parent will traverse them in the order they appear in the parent's children list. However, priorityOrder does not alter the layout and focus traversal order of this Node within its parent. A parent always traverses its children list in order when doing layout or focus traversal. Default value: 0.0 Since: 9 See Also: getPriorityOrder(), setPriorityOrder(double)
01-04-2016
1) Typo in the cssref: -fx-priorty-order 2) JavaDoc has the word 'till', which is not really a proper word (should be 'until'): "and so on, till the child with the lowest" 3) The Javadoc could possibly be made clearer still. It says that "The children are traversed in decreasing priorityOrder order." Perhaps it could be made clearer to the reader (so they don't have to go find documentation elsewhere) about whether this means the highest priority order will appear in front of or behind lower priority order siblings.
31-03-2016
I have updated the CSS reference doc and also discussed with Kevin on the implication of this feature on focus traversal. We concluded that focus traversal should be treated like layout ordering, they shouldn't be affected by Node.priorityOrder. I have reworded the doc. to explicitly mention "rendering and picking order" instead of "traversal order". Here is the proposed specification and implementation of Node.priorityOrder: http://cr.openjdk.java.net/~ckyang/JDK-8144556/webrev.00/
31-03-2016
1) Double is a better choice, for example a common use of this feature is to do transparency sorting and the data is often the computed distances (or one of the component in (x,y,z) coordinates of shapes) from the viewer. 2) Thanks! I will look into it. 3) Yes, this can be confusing if this feature shouldn't impact the focus traversal. I have yet to give this a thought.
18-03-2016
A few comments: 1) Why are you using double instead of integer? 2) You'll need to update the CSS reference doc to include the new property. 3) 'traversal order' in the javadoc might be unclear - I interpreted it to mean focus traversal (and indeed, there is a question about how or if this property should impact focus traversal).
17-03-2016
Here is a prototype of the implementation with a simple toy example if you wish to play with: Webrev: http://cr.openjdk.java.net/~ckyang/JDK-8144556/prototype/
17-03-2016
Please review the following new API proposal to add a priorityOrder property to Node. /** * Defines the traversal order of this {@code Node} within its parent. * * This property is used to alter the rendering and picking order of a node * within its parent without disturbing its child physical index, which makes * it useful for animating a node's ordering such as transparency sorting. * * The parent will traverse its children in a specific order. The children are * traversed in decreasing priorityOrder order. The child with the highest * priorityOrder is visited first, followed by the next higher priortyOrder * child, and so on, till the child with the lowest priorityOrder visited last. * * @defaultValue 0.0 * /
17-03-2016

Blocks :	JDK-8144684 - Incorrect blending of 3D-shapes with semi-transparent diffuse color
Relates :	JDK-8144907 - Support sorted children
Relates :	JDK-8094313 - Overlapping 3D transformed 2D shapes have dirty edges in Scene with depth buffer