<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"Apple Color Emoji";
panose-1:0 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:"Times New Roman \(Body CS\)";
panose-1:2 11 6 4 2 2 2 2 2 4;}
@font-face
{font-family:"Courier New \,serif";
panose-1:2 7 3 9 2 2 5 2 4 4;}
@font-face
{font-family:"Courier New \,serif ";
panose-1:2 7 3 9 2 2 5 2 4 4;}
@font-face
{font-family:"Iosevka Fixed SS16";
panose-1:2 0 5 9 3 0 0 0 0 4;}
@font-face
{font-family:"Courier New \,serif\;color\:black ";
panose-1:2 7 3 9 2 2 5 2 4 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:10.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
font-size:10.0pt;
font-family:"Calibri",sans-serif;}
p.p1, li.p1, div.p1
{mso-style-name:p1;
margin:0in;
font-size:9.0pt;
font-family:"Iosevka Fixed SS16";}
span.s1
{mso-style-name:s1;
color:#646464;}
span.s2
{mso-style-name:s2;
color:#4E8F00;}
span.EmailStyle23
{mso-style-type:personal-reply;
font-family:"Courier New",serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:319313581;
mso-list-template-ids:127296706;}
@list l0:level1
{mso-level-tab-stop:.5in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level2
{mso-level-tab-stop:1.0in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level3
{mso-level-tab-stop:1.5in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level4
{mso-level-tab-stop:2.0in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level5
{mso-level-tab-stop:2.5in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level6
{mso-level-tab-stop:3.0in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level7
{mso-level-tab-stop:3.5in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level8
{mso-level-tab-stop:4.0in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level9
{mso-level-tab-stop:4.5in;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1
{mso-list-id:1301495106;
mso-list-template-ids:-2036712714;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></style>
</head>
<body lang="EN-US" link="#0563C1" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16"">
<span style="color:#FF2600">/**</span><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * Sets the skin that will render this {@link Control}.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * <p><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * To ensure a one-to-one relationship between a {@code Control} and its<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * {@code Skin}, this method must check (for any non-null value) that<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * {@link Skin#getSkinnable()}, throwing an IllegalArgumentException<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * in the case of mismatch.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * returns the same value as this Skinnable.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> *<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * @param value the skin value for this control<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * <o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * @throws IllegalArgumentException if {@link Skin#getSkinnable()} returns<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> * value other than this Skinnable.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16";color:#FF2600"> */<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:"Iosevka Fixed SS16"">
<span style="color:#4E8F00">public</span> <span style="color:#4E8F00">void</span> setSkin(Skin<?> value);<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New",serif"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New",serif"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New",serif">-andy<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New",serif"><o:p> </o:p></span></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:12.0pt;color:black">From:
</span></b><span style="font-size:12.0pt;color:black">Kevin Rushforth <kevin.rushforth@oracle.com><br>
<b>Date: </b>Friday, 2022/07/22 at 14:54<br>
<b>To: </b>Andy Goryachev <andy.goryachev@oracle.com>, openjfx-dev@openjdk.org <openjfx-dev@openjdk.org>, Philip Race <philip.race@oracle.com><br>
<b>Subject: </b>Re: [External] : Aw: Proposal: Add Skin.install() method<o:p></o:p></span></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:11.0pt">Yes, but a RuntimeException only appears in the javadoc, and not in the method signature (only checked exceptions show up in the method signature).<br>
<br>
-- Kevin<br>
<br>
<o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt">On 7/22/2022 2:39 PM, Andy Goryachev wrote:<o:p></o:p></span></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif"">Thank you, Phil and Kevin.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif""> </span><o:p></o:p></p>
<p class="p1"><span class="s1">@Override</span><o:p></o:p></p>
<p class="p1"><span class="s2">public</span> <span class="s2">final</span> <span class="s2">
void</span> setSkin(Skin<?> value) throws IllegalArgumentException {<o:p></o:p></p>
<p class="p1"> <o:p></o:p></p>
<p class="p1"> <o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif"">-andy</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif""> </span><o:p></o:p></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:12.0pt;color:black">From:
</span></b><span style="font-size:12.0pt;color:black">Kevin Rushforth <a href="mailto:kevin.rushforth@oracle.com">
<kevin.rushforth@oracle.com></a><br>
<b>Date: </b>Friday, 2022/07/22 at 14:20<br>
<b>To: </b>Andy Goryachev <a href="mailto:andy.goryachev@oracle.com"><andy.goryachev@oracle.com></a>,
<a href="mailto:openjfx-dev@openjdk.org">openjfx-dev@openjdk.org</a> <a href="mailto:openjfx-dev@openjdk.org">
<openjfx-dev@openjdk.org></a><br>
<b>Subject: </b>Re: [External] : Aw: Proposal: Add Skin.install() method</span><o:p></o:p></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:11.0pt">No, sorry. Error doesn't communicate the right thing. This is exactly is what RuntimeException is intended to be used for. It's no different from passing an out of range numerical
value or a null to a method that doesn't take null. In all similar cases, the method will document the exception cases with '@throws' javadoc tags, and the developer can then understand that they passed something bad to the method. FWIW, I've never seen any
Java API throw Error in this manner.<br>
<br>
-- Kevin<br>
<br>
<br>
</span><o:p></o:p></p>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt">On 7/22/2022 2:02 PM, Andy Goryachev wrote:</span><o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">I do mean java.lang.Error.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">The goal is to prevent an incorrect code from being shipped to the end user. There are no tools at the API level to enforce the 1:1 relationship, so it cannot be checked
at compile time. </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">The next best thing is to fail during the development, thus an Error. It should be an error and not a RuntimeException because it communicates a design error, and not a
run time, i.e. a legitimate run time condition. It is also not an IllegalArgumentException because there should be no scenario when this could happen.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">In other words, the condition should get fixed by a redesign rather than by handling/ignoring an exception. As stated in the Error javadoc</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Arial",sans-serif;color:black;background:#ECEBEC">An </span><span style="font-size:8.5pt;font-family:"Courier New \,serif\;color\:black "">Error</span><span style="font-size:8.5pt;font-family:"Arial",sans-serif;color:black;background:#ECEBEC"> is
a subclass of </span><span style="font-size:8.5pt;font-family:"Courier New \,serif\;color\:black "">Throwable</span><span style="font-size:8.5pt;font-family:"Arial",sans-serif;color:black;background:#ECEBEC"> that indicates
<span style="background:yellow;mso-highlight:yellow">serious problems that a reasonable application should not try to catch</span>. Most such errors are abnormal conditions. The </span><span style="font-size:8.5pt;font-family:"Courier New \,serif\;color\:black "">ThreadDeath</span><span style="font-size:8.5pt;font-family:"Arial",sans-serif;color:black;background:#ECEBEC"> error,
though a "normal" condition, is also a subclass of </span><span style="font-size:8.5pt;font-family:"Courier New \,serif\;color\:black "">Error</span><span style="font-size:8.5pt;font-family:"Arial",sans-serif;color:black;background:#ECEBEC"> because most applications
should not try to catch it.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">if this idea seems to radical, I am ok with making it an IllegalArgumentException.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">-andy</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:12.0pt;color:black">From:
</span></b><span style="font-size:12.0pt;color:black">Kevin Rushforth <a href="mailto:kevin.rushforth@oracle.com">
<kevin.rushforth@oracle.com></a><br>
<b>Date: </b>Friday, 2022/07/22 at 13:42<br>
<b>To: </b>Andy Goryachev <a href="mailto:andy.goryachev@oracle.com"><andy.goryachev@oracle.com></a>,
<a href="mailto:openjfx-dev@openjdk.org">openjfx-dev@openjdk.org</a> <a href="mailto:openjfx-dev@openjdk.org">
<openjfx-dev@openjdk.org></a><br>
<b>Subject: </b>Re: [External] : Aw: Proposal: Add Skin.install() method</span><o:p></o:p></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:11.0pt">I don't know if you really meant Error, as in java.lang.Error, but it would need to be a subclass of RuntimeException. IllegalArgumentException seems the natural choice (a case
could possibly be made for IllegalStateException). Throwing an Error is not the right thing for a library to do in response to an application passing in an illegal or unexpected argument to a method or constructor. It is for truly exceptional things that a
programmer cannot anticipate (like running out of memory).<br>
<br>
-- Kevin<br>
<br>
<br>
<br>
</span><o:p></o:p></p>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt">On 7/22/2022 12:37 PM, Andy Goryachev wrote:</span><o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">I would rather throw an Error in Skinnable.setSkin() when mismatch is detected. This is a design error that should be caught early in development rather than a run time
exception.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">-andy</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:12.0pt;color:black">From:
</span></b><span style="font-size:12.0pt;color:black">openjfx-dev <a href="mailto:openjfx-dev-retn@openjdk.org">
<openjfx-dev-retn@openjdk.org></a> on behalf of Kevin Rushforth <a href="mailto:kevin.rushforth@oracle.com">
<kevin.rushforth@oracle.com></a><br>
<b>Date: </b>Friday, 2022/07/22 at 12:33<br>
<b>To: </b><a href="mailto:openjfx-dev@openjdk.org">openjfx-dev@openjdk.org</a> <a href="mailto:openjfx-dev@openjdk.org">
<openjfx-dev@openjdk.org></a><br>
<b>Subject: </b>Re: [External] : Aw: Proposal: Add Skin.install() method</span><o:p></o:p></p>
</div>
<p class="MsoNormal"><span style="font-size:11.0pt">I would not be in favor of adding a no-arg constructor to SkinBase, for the reasons Andy gave. Additionally, there would be no way to avoid braking the contract of Skin::getSkinnable which says:<br>
<br>
"This value will only ever go from a non-null to null value when the Skin is removed from the Skinnable, and only as a consequence of a call to dispose()."<br>
<br>
<br>
<br>
<br>
<br>
</span><o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">At the very minimum, we should explain in Skin javadoc that creating a skin for one control and setting it in the
other is a no-no. Or, perhaps we should explicitly check for this condition in setSkin().</span><o:p></o:p></p>
</blockquote>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:11.0pt"><br>
I agree completely. At a minimum this enhancement should change the docs for setSkin to say that a skin created for one control should not (must not?) be used in another control. And unless there is a legitimate use case I haven't thought of, I think we could
consider an explicit check, and either throw an Exception (this seems the best choice, unless there are compatibility concerns), or else log a warning and treat it as a no-op.<br>
<br>
-- Kevin<br>
<br>
<br>
<br>
<br>
</span><o:p></o:p></p>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt">On 7/22/2022 9:13 AM, Andy Goryachev wrote:</span><o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">You do bring a good point! I don't know the rationale behind passing control pointer to the Skin constructor.
</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">I think Swing got it right, clearly separating</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<ol style="margin-top:0in" start="1" type="1">
<li class="MsoListParagraph" style="margin-left:0in;mso-list:l0 level1 lfo3"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">instantiation (using either a no-arg constructor, or any other constructor that does not require component pointer)</span><o:p></o:p></li><li class="MsoListParagraph" style="margin-left:0in;mso-list:l0 level1 lfo3"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">configuration (optional step, possibly widely separated in time and space)</span><o:p></o:p></li><li class="MsoListParagraph" style="margin-left:0in;mso-list:l0 level1 lfo3"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">uninstallation of the old skin</span><o:p></o:p></li><li class="MsoListParagraph" style="margin-left:0in;mso-list:l0 level1 lfo3"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">installation of the new skin</span><o:p></o:p></li></ol>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">What you are proposing - moving to a default constructor makes the most sense. It comes with a high price though - everyone with a custom skin implementation would need
to change their code. </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">At the very minimum, we should explain in Skin javadoc that creating a skin for one control and setting it in the other is a no-no. Or, perhaps we should explicitly check
for this condition in setSkin().</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Thank you</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">-andy</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:12.0pt;color:black">From:
</span></b><span style="font-size:12.0pt;color:black">Marius Hanl <a href="mailto:mariushanl@web.de">
<mariushanl@web.de></a><br>
<b>Date: </b>Friday, 2022/07/22 at 05:06<br>
<b>To: </b><a href="mailto:openjfx-dev@openjdk.org">openjfx-dev@openjdk.org</a> <a href="mailto:openjfx-dev@openjdk.org">
<openjfx-dev@openjdk.org></a>, Andy Goryachev <a href="mailto:andy.goryachev@oracle.com">
<andy.goryachev@oracle.com></a><br>
<b>Subject: </b>[External] : Aw: Proposal: Add Skin.install() method</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt">I had a similar idea in the past and like the idea.<br>
Ideally, setting/switching a skin is a one step process. Currently you can construct a skin for a control and set it after to a different control.<br>
<br>
Your approach sounds good, if you can set a skin by creating a new skin (with a default constructor) and then the setSkin() method will actually trigger the install process on the control (this), this will work and solve the problem above. But for backward
compatibilty we still need to keep the skin constructor with the control as parameter and think about deprecating it.<br>
<br>
-- Marius</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt">Am 20.07.22, 23:40 schrieb Andy Goryachev
<a href="mailto:andy.goryachev@oracle.com"><andy.goryachev@oracle.com></a>:</span><o:p></o:p></p>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:4.8pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Hi,</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">I'd like to propose an API change in Skin interface (details below). Your feedback will be greatly appreciated!</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Thank you,</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">-andy</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Summary</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">-------</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Introduce a new Skin.install() method with an empty default implementation. Modify Control.setSkin(Skin) implementation to invoke install() on the new skin after the old
skin has been removed with dispose().</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Problem</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">-------</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Presently, switching skins is a two-step process: first, a new skin is constructed against the target Control instance, and is attached (i.s. listeners added, child nodes
added) to that instance in the constructor. Then, Control.setSkin() is invoked with a new skin - and inside, the old skin is detached via its dispose() method. </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">This creates two problems:</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> 1. if the new skin instance is discarded before setSkin(), it remains attached, leaving the control in a weird state with two skins attached, causing memory leaks and performance
degradation.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> 2. if, in addition to adding listeners and child nodes, the skin sets a property, such as an event listener, or a handler, it overwrites the current value irreversibly.
As a result, either the old skin would not be able to cleanly remove itself, or the new skin would not be able to set the new values, as it does not know whether it should overwrite or keep a handler installed earlier (possibly by design). Unsurprisingly,
this also might cause memory leaks.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">We can see the damage caused by looking at
<a href="https://bugs.openjdk.org/browse/JDK-8241364">JDK-8241364</a> </span><span style="font-size:11.0pt;font-family:"Apple Color Emoji"">☂</span><span style="font-size:11.0pt;font-family:"Courier New \,serif "">
<i>Cleanup skin implementations to allow switching</i>, which refers a number of bugs:</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8245145 Spinner: throws IllegalArgumentException when replacing skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8245303 InputMap: memory leak due to incomplete cleanup on remove mapping</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8268877 TextInputControlSkin: incorrect inputMethod event handler after switching skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8236840 Memory leak when switching ButtonSkin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8240506 TextFieldSkin/Behavior: misbehavior on switching skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8242621 TabPane: Memory leak when switching skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8244657 ChoiceBox/ToolBarSkin: misbehavior on switching skin
</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8245282 Button/Combo Behavior: memory leak on dispose</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8246195 ListViewSkin/Behavior: misbehavior on switching skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8246202 ChoiceBoxSkin: misbehavior on switching skin, part 2</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8246745 ListCell/Skin: misbehavior on switching skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8247576 Labeled/SkinBase: misbehavior on switching skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8253634 TreeCell/Skin: misbehavior on switching skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8256821 TreeViewSkin/Behavior: misbehavior on switching skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8269081 Tree/ListViewSkin: must remove flow on dispose</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8273071 SeparatorSkin: must remove child on dispose</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8274061 Tree-/TableRowSkin: misbehavior on switching skin</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8244419 TextAreaSkin: throws UnsupportedOperation on dispose</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">JDK-8244531 Tests: add support to identify recurring issues with controls et al</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Solution</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">--------</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">This problem does not exist in e.g. Swing because the steps of instantiation, uninstalling the old ComponentUI ("skin"), and installing a new skin are cleanly separated.
ComponentUI constructor does not alter the component itself, ComponentUI.uninstallUI(JComponent) cleanly removes the old skin, ComponentUI.installUI(JComponent) installs the new skin. We should follow the same model in javafx.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Specifically, I'd like to propose the following changes:</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> 1. Add Skin.install() with a default no-op implementation.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> 2. Clarify skin creation-attachment-detachment sequence in Skin and Skin.install() javadoc</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> 3. Modify Control.setSkin(Skin) method (== invalidate listener in skin property) to call oldSkin.dispose() followed by newSkin.install()</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> 4. Many existing skins that do not set properties in the corresponding control may remain unchanged. The skins that do, such as TextInputControlSkin (JDK-8268877), must
be refactored to utilize the new install() method. I think the refactoring would simply move all the code that accesses its control instance away from the constructor to install().</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">Impact Analysis</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">-------------</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">The changes should be fairly trivial. Only a subset of skins needs to be refactored, and the refactoring itself is trivial. </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">The new API is backwards compatible with the existing code, the customer-developed skins can remain unchanged (thanks to default implementation). In case where customers
could benefit from the new API, the change is trivial.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif "">The change will require CSR as it modifies a public API.</span><o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Courier New \,serif ""> </span><o:p></o:p></p>
</blockquote>
</div>
</blockquote>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><o:p></o:p></p>
</blockquote>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><o:p></o:p></p>
</blockquote>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><o:p></o:p></p>
</blockquote>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
</div>
</body>
</html>