Skip to content
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

Prioritize print-color-mode-supported over PWG when creating color mo… #1

Merged
merged 1 commit into from
Oct 26, 2020
Merged

Prioritize print-color-mode-supported over PWG when creating color mo… #1

merged 1 commit into from
Oct 26, 2020

Conversation

zdohnal
Copy link
Member

@zdohnal zdohnal commented Sep 14, 2020

…dels

See apple/cups#5722

Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the reasons for this change, as print-color-mode doesn't actually convey color space support for a printer, just the basic mono-vs-color choice at print time. Why was this change made in cups-filters?

urf-supported is an AirPrint attribute and pwg-raster-document-type-supported is for IPP Everywhere/Wi-Fi Direct/Mopria support - seems like only having print-color-mode-supported will be the odd use case.

@michaelrsweet michaelrsweet added enhancement New feature or request priority-low labels Oct 14, 2020
@zdohnal
Copy link
Member Author

zdohnal commented Oct 19, 2020

@michaelrsweet It was introduced due problems with at least two Canon printers (Canon ImageRunner Advance at the office and Canon MF645Cx from apple/cups#5713), where pwg-raster-document-type-supported attribute contained only srgb_8, resulting into the problem created PPD didn't contain an option to change color mode into grayscale.
I was considering creating a 'template', meaning if pwg-raster-document-type-supported does provide colored option, but does not grayscale one, just add a grayscale one automatically, but we ended up with this solution.

Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the description of the issue in the conversation, I think I would prefer having the code look for sgray_8, black_1, or black_8 when it sees srgb_8, and synthesize sgray_8 if there is no grayscale color space listed. The reason is that print-color-mode is effectively a rendering intent while pwg-raster-document-type-supported lists supported color spaces like AdobeRGB, sRGB, CMYK, etc. which are important for photo printing in particular.

@michaelrsweet michaelrsweet added the platform issue Issue is specific to an OS or desktop label Oct 19, 2020
@zdohnal
Copy link
Member Author

zdohnal commented Oct 26, 2020

The code is updated - I used ippContainsString() for checking for grayscale color models, and used ippGetCount() to check whether there is only one keyword. I thought it could speed up the execution if there is only one keyword.

Our Canon printer at the office got firmware update, which fixed the issue (got now srgb_8 is listed twice in pwg-document-format-supported :( , but I would let it go and report it to Canon), so I'm not able to verify whether it works when only srgb_8 is defined, but it doesn't generate a duplicate Grayscale entry at least and IMO there can be still printers in the wild with this error, so merging it makes sense.

There was an other reporter, which hit the issue with Canon, so I'll try to synch with him and see if we can verify the fix.

Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll drop the check for count == 1 but otherwise I like the new fix much better!

cups/ppd-cache.c Show resolved Hide resolved
@michaelrsweet michaelrsweet merged commit 374bf88 into OpenPrinting:master Oct 26, 2020
michaelrsweet added a commit that referenced this pull request Oct 26, 2020
@michaelrsweet michaelrsweet added this to the v2.3.3op1 milestone Nov 22, 2020
michaelrsweet pushed a commit that referenced this pull request Mar 15, 2021
updating from openprinting paster
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request platform issue Issue is specific to an OS or desktop priority-low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants