On Fri, 14 Mar 2025 01:24:04 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>>> > Since there is no way to check if a profile is built-in, making built-in >>> > profiles read-only might be a better approach, similar to [java >>> > properties](https://bugs.openjdk.org/browse/JDK-8066709). >>> >>> But it also has its own disadvantages... >> >> @mrserb You mentioned earlier that there are disadvantages of using java >> properties approach, so using builtIn flag would be better then, correct? >> >>> I remember we discussed this issue during the review of [this >>> commit](https://github.com/openjdk/jdk/pull/3037/files#diff-3dd53a33889801159f43dbb990ba033066bdabaed71bbc7254f58331d3898d69R39), >>> and the conclusion was that it was too late to change it and break apps. >> >> I wasn't able to find the previous discussion thread so I'm not sure what >> are the disadvantages. Can you please explain? > > If we start from the beginning. The problem we are trying to solve is > preventing changes to our standard profiles so that we can be sure one part > of the application does not unexpectedly affect another. This could be > achieved in two ways: > > - Throw an exception if a standard profile is modified > - Ignore the requested change and use the default profile properties as is > > For new applications, we could require always cloning the color profile > before modification (via the updated specification); otherwise, the > application must always check whether the profile is built-in or not. > > However, for older applications, these two solutions lead to different > outcomes: > > - Throwing the new exception will most likely break the application > - Ignoring the data change allows the application to continue working, but > the color conversion result may not be fully accurate > > The next question is: how big of an issue is this inaccuracy? For example, if > we have an sRGB profile that rejects some modifications, we would still try > to produce valid sRGB data during conversion. > > So the trade-off is: broken applications vs. inaccurate (but still > ICC-spec-compliant) conversions. > > PS: By the way, why do we want to enforce this rule only for built-in > profiles? Why can't a library create similar profiles and mark them as > "read-only"? @mrserb > The app may obtain a profile from some place and use it for some images or > pixels. Then, if the app wants to tweak the rendering intent for some reason, > what should it do? > > Clone the profile and then change the intent? The app would follow the above approach. There is only one step extra with this fix - creating a copy of built-in profile and then modifying it. byte[] builtInData = ICC_Profile.getInstance(ColorSpace.CS_sRGB).getData(); // get the byte array representation of BuiltIn- profile ICCProfile newProfile = ICC_Profile.getInstance(builtInData) // create a new profile newProfile.setData(...) > However, for older applications, these two solutions lead to different > outcomes: > Throwing the new exception will most likely break the application The chances of breaking an existing application is minimal since API usage for setData() was checked and not many were found and moreover they were not called on built-in profiles. >Ignoring the data change allows the application to continue working, but the >color conversion result may not be fully accurate This does not seem viable because the user in unaware as to whether setData() worked or not. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r2001540142
