On Mon, 24 Feb 2025 20:04:26 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
>>> There are other way to create a profile - directly loading it form a file >>> (serialization) `ICC_Profile.getInstance(<path to sRGB.pf or any custom >>> profiles>); `or using the byte array representation of the profile. So the >>> main intention here was not to tie ProfileDeferralInfo with isBuiltIn. >> >> Yes, there are. Does any other way create a **built-in profile**? No, it >> doesn't as far as I can see. >> >> 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. > >> 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1976123437
