Ok, thanks for elaborating. I'll submit a PR for review soon. Take care,
Daniel On Fri, Aug 22, 2025 at 7:11 AM Philip Race <philip.r...@oracle.com> wrote: > I suppose they won't be that surprised, since it has been the behaviour > already. > > I think it unlikely that an app throws null at TextLayout on purpose > knowing it won't work. > That's the realm of conformance tests, not real programs, so I'm not > terribly worried about that. > > What I was getting at, in part, is that it seems possible to me that this > was not checked > because the thinking was in this case, null was allowed and meant a > default FRC. > If we change the exception type it makes it one step harder if we find out > that's where we want to end up. > I don't see a lot of supporting evidence for that, so I am just throwing > it out there. > It could also be that it was added at a different time by some one not > following the rest of the API > > But this point is minor. The big question is around the empty string. > It certainly was a pain for me when doing printing when I had needed to > use TextLayout > and had to be careful of this case. > > It would be 'ideal' if "" was just silently handled and didn't cause > issues for existing code - other than conformance tests > which shouldn't be testing unspecified behaviour anyway. > > -phil. > > On 8/19/25 4:29 AM, Daniel Gredler wrote: > > Hi Phil, > > >> First, there is a related API oddity in that null FontRenderContext > >> parameters passed to the TextLayout constructors cause a > >> NullPointerException, rather than an IllegalArgumentException (like > >> all other parameters). Can this behavior also be changed? > > > It is a bit odd that frc is the only one not explicitly checked so the > > NPE is just what 'happens'. > > I'd be reluctant to change the NPE without a good reason [...] > > I'd say just specify what happens for complete compatibility. > > I guess I'm hesitant to promote an "accidental" NPE to official behavior > here, when all other null parameter values are handled with an explicit > IAE. This inconsistency is going to surprise developers, and avoiding that > surprise would be ideal. How likely is it that adding an explicit > IAE-generating null FRC check here will break existing code? > > Take care, > > Daniel > > > On Mon, Aug 11, 2025 at 7:12 PM Philip Race <philip.r...@oracle.com> > wrote: > >> >> >> On 8/11/25 9:20 AM, Daniel Gredler wrote: >> > Hi all, >> > >> > I was thinking of taking a stab at JDK-4138921 [1], and I have a >> > couple of questions. >> > >> > First, there is a related API oddity in that null FontRenderContext >> > parameters passed to the TextLayout constructors cause a >> > NullPointerException, rather than an IllegalArgumentException (like >> > all other parameters). Can this behavior also be changed? >> >> None of these are documented .. they all should be. >> >> It is a bit odd that frc is the only one not explicitly checked so the >> NPE is just what 'happens'. >> I'd be reluctant to change the NPE without a good reason and I'm >> half-wondering if a null FRC was >> supposed to default to a default FRC ?? But somewhere along the line the >> implementation changed. >> I'd say just specify what happens for complete compatibility. >> >> > >> > Second, changing all three TextLayout constructors to accept empty >> > strings sort of implies that we should also allow empty strings in >> > AttributedString instances (this is currently only allowed if the >> > attribute map is empty). Is it OK to change this behavior as well? >> >> I don't think I ever understood why this was dis-allowed on TextLayout. >> Perhaps it was to prevent some incorrect usage from ever slipping into >> being acceptable ? >> >> AttributedString is part of the java.base module, I don't have any say >> over that, although in the very beginning >> there was a decent overlap in the people working on that and TextLayout >> etc. >> I'd start by asking Naoto (cc'ed). >> >> -phil. >> >> > >> > Let me know your thoughts! >> > >> > Daniel >> > >> > [1] https://bugs.openjdk.org/browse/JDK-4138921 >> > >> > >> >> >
