On Fri, 28 Feb 2025 23:44:40 GMT, Phil Race <p...@openjdk.org> wrote:
>>> Yes, there are. Does any other way create a built-in profile? No, it >>> doesn't as far as I can see. >> >> Yes, currently ProfileDeferralInfo is the only way to create built-in >> profile. >> >>> Is this flexibility needed? I'd say, it's not needed… unless there's a very >>> high chance there'll soon be introduced a new build-in ICC profile which is >>> created in another way but ICC_Profile(ProfileDeferralInfo) constructor. >> >> As of now we don't need the flexibility but not sure if that is something >> that we may require or need to consider in future. @prrace Your suggestion ? > > So in fact Harshitha had prototyped the constructor mechanism - but it was > passing a parameter so wasn't as small a diff as the most recent alternate, > because I think it rippled into changing the sub-class constructors as well, > which made it less desirable. > Another particular concern was that it was equating ProfileDeferralInfo to > meaning isBuiltIn == true, which is the case today, but it is not a > fundamental truth. > And the proposal here is more direct and you don't have to go follow the > bread crumbs to see exactly which profiles have this set to true. So that is > why the PR is as it is. > Neither is "wrong" - they are just different choices based on different > perceptions. > > But if we can do this with changes to *just this class*, then may be .. and > since I see the sub-class constructors (for RGB and Gray) call the same > signature super-class constructors, then I think this should work, and it > will be OK. > I tried it on a local repo, and came up with the same changes as suggested > and it least builds .. > So we can do it that way too, but there should be a comment somewhere that > ProfileDeferralInfo is only for built-in profiles - and that in fact all > built-in profiles should use it. Thanks for the update on this discussion Phil. I'll update the code to set built-in flag in the constructor and add a comment stating - ProfileDeferralInfo to be used only for built-in profiles. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1976131267
