[API Review]: Make SubScene Resizable (RT-31377)

[Pavel Safrata]

Richard?

On 18.9.2013 17:16, Pavel Safrata wrote:
> Hi Richard,
> do I understand correctly that you prefer replacing the existing 
> constructors by the following?
>
> public SubScene(Parent root, double width, double height, boolean 
> resizable, boolean depthBuffer, boolean antiAliasing)
> public SubScene(Parent root, double width, double height, boolean 
> resizable)
>
> Thanks,
> Pavel
>
> On 18.9.2013 16:59, Richard Bair wrote:
>> I think it is better to be explicit in all cases.
>>
>> On Sep 18, 2013, at 7:47 AM, Pavel Safrata <pavel.safrata at oracle.com> 
>> wrote:
>>
>>> Thanks, guys. Let me stay focused on the SubScene for now. Looks 
>>> like #2 wins. After some more thinking it turns out the boolean 
>>> isResizable constructor argument is not ideal because giving size to 
>>> a resizable subscene doesn't make much sense (it is resized as soon 
>>> as it is placed in layout so the initial size is not really used). 
>>> So I think it would be better to add constructors without the size 
>>> and set the resizability implicitly according to the constructor 
>>> used. It might not be immediately clear but to me it still seems 
>>> better than requiring two useless arguments.
>>>
>>> To make this more formal API review, I propose the following:
>>>
>>> The existing constructors will stay the same and continue to create 
>>> a fixed-size subscene:
>>> public SubScene(Parent root, double width, double height, boolean 
>>> depthBuffer, boolean antiAliasing)
>>> public SubScene(Parent root, double width, double height)
>>>
>>> Add new constructors creating a resizable subscene:
>>> public SubScene(Parent root, boolean depthBuffer, boolean antiAliasing)
>>> public SubScene(Parent root)
>>>
>>> The resizable subscene will forward root's min/pref/max sizes, the 
>>> non-resizable subscene will continue to use the user-set 
>>> width/height property value.
>>>
>>>
>>> Just for completness, the other option is to replace the long 
>>> constructor with
>>> public SubScene(Parent root, double width, double height, boolean 
>>> resizable, boolean depthBuffer, boolean antiAliasing)
>>> and document the width and height arguments to be of virtually no 
>>> use in the resizable case. Let me know if you'd prefer this option.
>>>
>>> Thanks,
>>> Pavel
>>>
>>> On 17.9.2013 16:59, Richard Bair wrote:
>>>> Personally I wish that it were possible to use pattern #2 with 
>>>> Rectangle, ImageView, and a bunch of others as well. Anything that 
>>>> *could* be resizable should have an option to be resizable. Heck, I 
>>>> wish it were possible to turn resizable on/off dynamically for 
>>>> SubScene or the others, not just an immutable property.
>>>>
>>>> But an immutable property with a constructor to set it is a good 
>>>> first step (especially since we don't have time in this release to 
>>>> do anything more comprehensive or radical).
>>>>
>>>> Richard
>>>>
>>>> On Sep 17, 2013, at 1:15 AM, Pavel Safrata 
>>>> <pavel.safrata at oracle.com> wrote:
>>>>
>>>>> Hello,
>>>>> we want to make SubScene resizable (reporting min/pref/max sizes 
>>>>> according to its root) for it to behave nicely when placed in 
>>>>> layout ( https://javafx-jira.kenai.com/browse/RT-31377 ). For the 
>>>>> main driver of SubScene's existence - 2D overlays over 3D content 
>>>>> - this makes perfect sense. However, there are use-cases where the 
>>>>> fixed size is needed. Mainly, every SubScene with 3D content 
>>>>> probably wants the fixed size as the content bounds are not really 
>>>>> meaningful after the perspective projection. So, we need to 
>>>>> support both resizable and non-resizable SubScene.
>>>>>
>>>>> There are two basic options:
>>>>>
>>>>> 1. Follow the pattern used in layouts. As SubScene is not a layout 
>>>>> class (doesn't inherit from Region), this would mean adding the 
>>>>> six methods ( set{Min|Pref|Max}{Width|Height} ) and duplicating 
>>>>> the Region's USE_COMPUTED_SIZE constant.
>>>>>
>>>>> + consistent with layouts
>>>>> - duplicated API
>>>>> - user needs six calls to make sure the SubScene has fixed size
>>>>>
>>>>> 2. Add an isResizable constructor argument, then just make the 
>>>>> SubScene report root's min/pref/max sizes in the resizable case.
>>>>>
>>>>> + easy to use the SubScene in the fixed-size manner (and 
>>>>> resizable, too)
>>>>> + small API change
>>>>> - probably an unfamiliar pattern we don't have elsewhere (but, 
>>>>> SubScene is a pretty unique node)
>>>>>
>>>>> What do you think?
>>>>>
>>>>> Thanks,
>>>>> Pavel
>>>>>
>