-
-
Notifications
You must be signed in to change notification settings - Fork 699
colour: use suggested rendering intent as fallback #4347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really useful, thanks Kleis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we should change the default intent. This could change the behaviour of a lot of old code.
libvips/colour/icc_transform.c
Outdated
@@ -785,7 +789,7 @@ vips_icc_class_init(VipsIccClass *class) | |||
static void | |||
vips_icc_init(VipsIcc *icc) | |||
{ | |||
icc->intent = VIPS_INTENT_RELATIVE; | |||
icc->intent = VIPS_INTENT_AUTO; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't changing the default break a lot of old code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't encountered any issue with this in the test suite or with various sample images tested locally. The benefit of this approach is that processing XYB JPEG images will work out-of-the-box. Otherwise, you might get a green-ish image if the embedded profile is ignored, along with a warning saying: profile does not support relative input intent
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about picking the user's requested rendering intent, and if that's not available, falling back to the profile's suggested intent? Then only raising an error if both are missing.
We wouldn't need the AUTO setting, and libvips behaviour with old code would not change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! I've incorporated this in the latest changeset and also targeted the 8.16
branch instead.
When the requested rendering intent is not supported by the profile, fallback to the profile's suggested intent. An error will only be raised if the suggested intent is also unsupported. Resolves: libvips#3475.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woo!
When the requested rendering intent is not supported by the profile, fallback to the profile's suggested intent. An error will only be raised if the suggested intent is also unsupported.
Resolves: #3475.
Targets the 8.16 branch.