On Mon, 24 Feb 2025 18:58:14 GMT, Alexey Ivanov <[email protected]> wrote:
>> I don't get the reason why it's bad to modify the constructors.
>>
>> At this time, all the built-in profiles are created with
>> `ProfileDeferralInfo`.
>>
>> If `ProfileDeferralInfo` is used to create only built-in profiles, then
>> modifying `ICC_Profile(ProfileDeferralInfo pdi)` is enough:
>>
>>
>> ICC_Profile(ProfileDeferralInfo pdi) {
>> deferralInfo = pdi;
>> isBuiltIn = true;
>> }
>>
>>
>> And this is a *minimal* change.
>>
>> When the `isBuiltIn` is final, it's much easier to find where the value gets
>> set.
>
>> At this time, all the built-in profiles are created with
>> `ProfileDeferralInfo`.
>>
>> If `ProfileDeferralInfo` is used to create only built-in profiles, then
>> modifying `ICC_Profile(ProfileDeferralInfo pdi)` is enough.
>
> I've updated my branch with this change: `ICC_Profile(ProfileDeferralInfo
> pdi)` sets the `isBuiltIn` flag to `true`; `ICC_Profile(Profile p)` sets
> `isBuiltIn` to `false`.
>
> The diff to ‘master’:
> https://github.com/openjdk/jdk/compare/2a5d1da3355a4df3109ec42646b5b0cf088b4c2a..a34f16860c2f7d393f4a1fb57f46fba1a68a8412
>
> Only `ICC_Profile.java` is modified, which is *minimal*. Only several lines
> are added.
>
> If there's another way to create a built-in profile in the future, this
> approach will need to updated to take into account the new way. We'll deal
> with it at that time in the future.
>
> The diff to your branch:
> https://github.com/openjdk/jdk/compare/8da82c10b5be3b92655abef95fd5be0c8b6eaa00..a34f16860c2f7d393f4a1fb57f46fba1a68a8412
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1968253349
