API REVIEW REQUEST: Public API for Backgrounds and Borders on Region

[Jonathan Giles]

Rich,

The API looks fine. I only have small comments about minor issues:

  * I agree with John - it is worth reviewing the hashCode code. I'm not
    sure it is worth pre-computing everything given the cost of that,
    but there is a risk that you'll (very infrequently) end up with a
    hashCode for an object actually being zero, and in this case you'll
    always be running the slow path through hashCode(). Perhaps you can
    simply introduce a boolean to note whether hashCode has been computed.
  * In BorderRadii, I'm not overly familiar with the terminology of some
    of the methods in this class. For example,
    getTopLeftVerticalRadius(). I wonder if the JavaDoc can be more
    useful than "The length of the vertical radii of the top-left
    corner."? Of course, I could just become more well-versed in the CSS
    specifications :-)
  * In BorderRadii, I understand that the radii can be interpreted as
    either a value or a percentage, but I'm not fond of the method name
    you use to expose this, e.g. isTopLeftVerticalRadiusAsPercentage().
    It is the 'As' that bothers me - I would have rather had an 'A', but
    I can see why you chose to use 'As' - so that we don't end up with
    isTopLeftVerticalRadiusAPercentage() (which is actually less
    readable, even if it is better English). Personally, I would still
    use 'A' rather than 'As', but I'm not going to pursue it any further
    as it is a gut feel choice (and you've used 'As' throughout the
    Region API).

-- Jonathan

On 26/08/2012 6:36 a.m., Richard Bair wrote:
> Hi,
>
> I have been working on some API for Java 8, which would expose as public API what has been private since we released JavaFX 2.0 (with modification). There has been @treatasprivate API on Region since 2.0 and this has to be removed & replaced. One reason is that as part of the Java 8 release requirements we're supposed to provide all the API our Controls use in their implementation as public API so that any 3rd party Control can be written using all public API (ie: all our Controls need to rely on public API instead of private internal hacks). Second, as we move towards proposing a JSR, we're obviously going to have to remove & replace all the impl_ @treatasprivate details with public or private API since the impl_ methods will never make it through a JSR into the standard.
>
> As such, I have filed: http://javafx-jira.kenai.com/browse/RT-24506. I have attached to the issue the new API / implementations.
>
> As you can see, there are a fair number of new classes! They are all immutable, meaning that they can be cached / reused among multiple instances. So for example, you can reuse the same Background or Border among multiple Regions. This is obviously a good thing when it comes to performance, because we can reduce the number of Background / Border instances in the system at any one time, since nearly every control is going to reuse one that already exists. On the other hand, the downside to this is that when animating between two styles (unless you do a cross fade animation), you would have to create lots of intermediate objects.
>
> Having immutable objects also allows for some very good performance optimizations around caching. For example, while picking, we presently have to recompute the insets / outsets of all borders and backgrounds to determine whether the region is picked or not (if pickOnBounds is false). Now however, we can cache the answer once on the Background and it is a very fast operation to determine whether we picked or not.
>
> On the rendering side, we can maintain a simple cache and use the Background as the key. Instead of rendering 5 backgrounds every time, we can render them once to a small image and use 9-patch rendering to scale the image and resize it and such.
>
> If they were instead mutable objects, none of those techniques would be possible AND every region that used a Background would have to register listeners (or vice-versa) so that when the Background (or one of its properties deep inside it) changed, the Region could be marked invalid and re-drawn. Mutability would be a major issue in this case.
>
> Please do have a look. There are a LOT of classes here, but if you have these classes in one hand and the CSS 3 spec in the other, hopefully you will see how we map the CSS 3 spec (and our existing CSS Reference for JavaFX) onto these classes and have provided for all of the (many, many, many) switches and twiddles that the spec allows for.
>
> Thanks
> Richard