[API review] Relevant to RT-26277. Affects Region layout. Your input is needed.
David Grieve
david.grieve at oracle.com
Wed Nov 20 11:43:21 PST 2013
The purpose of this email is to solicit feedback on an issue related to Region layout and the use of percent values in insets. https://javafx-jira.kenai.com/browse/RT-26277 is the relevant JIRA issue.
Currently, a Region's width and height depends on the width and height of its content plus the Region's insets. At issue here is the reliance on border insets and to calculate the Region's insets if the border insets are allowed to use percentage values. The reason this matters to the community is that resolving this issue will require adding new API and changes to API added in 8.0. This may also cause regressions in CSS styles.
Region is a rough approximation of the W3C CSS box model (http://www.w3.org/TR/CSS2/box.html). The main difference is that Region doesn't have margins (this is an oversight which is called out in https://javafx-jira.kenai.com/browse/RT-27785).
In current code, the width and height of a Region is calculated as the content width/height plus the width/height of the Region's insets as returned by Region#getInsets(). The getInsets() method returns the Region's padding plus the border insets as returned by Border#getInsets(). Allowing percent values in border insets (via -fx-border-insets) causes problems for this calculation. If percent values are allowed, then the percent values should be relative to the width and height of the Region's parent. This is consistent with the CSS box model. The problem, and this is a problem for the box model as well, is that the width and height of the Region's parent depends on the width and height of the Region itself. This is not true if the Region is unmanaged, but that would be atypical.
Outlined below are changes that allow backgrounds and borders to have percentage based insets. Part of the solution is to add margin insets into Region. These changes would allow percentage values to be used for Region's margin and padding which, as noted, is an issue. In a sense, these changes just displace the problem of calculating width and height when there are percentage values in the insets. I've not tried to solve this particular problem and I assert that it does not need to be solved in this release. But without the changes outlined below, the issue of percent values in background and borders insets cannot be resolved in the future without incompatible changes or some really ugly API.
These changes also have the potential to cause regressions in the visual affects of CSS styles, especially where positive valued border insets have been used. For example, if some style insets the border on the left and top by one pixel, existing code will add that 1 pixel to the Region's width and height. With this change, the Region's width and height would no longer incorporate the border's insets. This would be resolved by adding appropriate margin to the Region (1px left and top). There are some cases of this in modena.css and caspian.css. A question for the community is whether or not this is a concern.
What I'd like to propose first is to change Region's getInsets such that it no longer depends on border. If Region no longer depends on Border for the Region's insets, the border insets can take on percentage values which can be normalized from the Region's parent's width and height.
Region API additions:
/** styleable from CSS via -fx-margin */
public final ObjectProperty<Insets> marginProperty()
This would allow a Region's width and height to be calculated from the margin width/height plus padding width/heigh plus content width/height. Note that this is not consistent with the box model which also includes the border width/height in the calculation. The justification for leaving it out of Region's width/height calculations is that JavaFX allows the border to be drawn outside the bounds of the Region. The border is still important for calculating geometric bounds, but not for layout bounds. I would really like to get some thoughts and opinions on this point.
The second thing I would like to propose is to add flags to Insets to tell whether or not the inset values represent percentages. This would allow insets to be given as percentages in css. This is a backward-compatible API change. The other alternative would be to add a new insets class that allows percentage values, but this proves difficult because much of the API already uses Insets.
Insets API additions:
/** return true if the top inset is a percentage, etc. */
public boolean isTopAsPercentage()
public boolean isRightAsPercentage()
public boolean isBottomAsPercentage()
public boolean isLeftAsPercentage()
/** if top is a percentage, return top*height, otherwise top, etc. */
public double getTop(double height)
public double getRight(double width)
public double getBottom(double height)
public double getLeft(double width)
/** return a new Insets with values normalized to pixels */
public Insets normalize(double width, double height)
The downside to this API is that it makes Insets more difficult to use since there are already parameterless getTop/Right/Bottom/Left methods and one has to be careful to know whether the return value represents a percentage or not.
An alternative would be to create a RegionInsets class that is the same as Insets except that the getters would all require a width or height parameter. The upside is that there would be no confusion over which get method to use. The downside is that much of the existing API uses Insets and RegionInsets would ultimately need to be converted to Insets.
This Insets API requires that width/height be passed everywhere Insets are used unless, like Region getInsets(), the insets are known to be normalized. This means that the Background class and the Border class need to have API changes. Both Background and Border are new API in 8.0 so there are no issues with backward compatibility.
Background API changes:
- public final Insets getOutsets()
+ public final Insets getOutsets(double width, double height)
Border API changes:
- public final Insets getOutsets()
+ public final Insets getOutsets(double width, double height)
- public final Insets getInsets()
+ public final Insets getInsets(double width, double height)
Also, BorderImage and BorderStroke would no longer be able to pre-compute their inner and outer edge. This is package API, but a change worth noting here:
BorderImage and BorderStroke API additions:
+ final Insets getInnerEdge(double width, double height)
+ final Insets getOuterEdge(double width, double height)
More information about the openjfx-dev
mailing list