[OpenJDK 2D-Dev] [10] RFR JDK-8186987:NullPointerException in RasterPrinterJob without PrinterResolution

[Prasanta Sadhukhan]

Hi Phil,


On 9/19/2017 3:16 AM, Phil Race wrote:
> Hi,
>
>
> On 09/04/2017 03:07 AM, Prasanta Sadhukhan wrote:
>> Hi Prahalad, Phil,
>>
>> JDK uses DocumentProperties function [1] to query the printer 
>> configuration data in windows.
>>
>> For a RIcoh printer which we (in IDC) use by default, 
>> pDevMode->dmFields is initialized to 0x200ff0f, which corresponds to
>> DM_MEDIATYPE  | DM_DUPLEX   | DM_YRESOLUTION   | DM_TTOPTION | 
>> DM_COLLATE  | DM_COPIES  | DM_DEFAULTSOURCE  | DM_PRINTQUALITY |
>> DM_COLOR  | DM_PAPERWIDTH  | DM_PAPERLENGTH | DM_PAPERSIZE  | 
>> DM_ORIENTATION
>>
>> But, for, Brother printer, pDevMode->dmFields is initialized to 
>> 0x1930f which corresponds to
>> DM_FORMNAME  | DM_DUPLEX   | DM_COLLATE | DM_COPIES  | 
>> DM_DEFAULTSOURCE  | DM_PAPERWIDTH  | DM_PAPERLENGTH | DM_PAPERSIZE | 
>> DM_ORIENTATION
>> so there is no DM_YRESOLUTION and DM_PRINTQUALITY in dmFields,
>> so even though YRESOLUTION and PRINTQUALITY is populated by 
>> :DocumentProperties API, the corresponding indices are not set,
>> resulting in having "GETDEFAULT_ERROR" [-50] in the array for those 
>> indices.
>>
>
> Very odd. It sounds like a driver bug and I'm surprised that the 
> Microsoft HWQL tests don't catch it.
> Was the driver directly from Brother ? 
Yes, I took the driver from 
http://support.brother.com/g/b/downloadlist.aspx?c=us_ot&lang=en&prod=hl2240d_all&os=93

> Perhaps someone considered those fields so mandatory
> that you don't even need to check that they are set ?
>
> This case reminds me of the fishy settings for a different bug 
> reviewed here : 
> http://mail.openjdk.java.net/pipermail/2d-dev/2016-June/007011.html so 
> ... I suppose this might be OK but still ..
>
>> I have modified my fix to populate the yresolution and printquality 
>> indices even though dmFields are not set by :DocumeProperties, 
>> provided those fields are not 0.
>
>
> Actually the code says you are doing that only if as they are GREATER 
> than zero .. not just different than zero.
>
> So here
> 919 if (pDevMode->dmFields & DM_PRINTQUALITY || 
> pDevMode->dmPrintQuality > 0) { If quality is used by a driver but it 
> forget to set the mask bit then you will still ignore it. Perhaps you 
> want != 0 ?
Yes, right. I actually overlooked the spec which says

*dmPrintQuality*

    Specifies the printer resolution. There are four predefined
    device-independent values:

    *DMRES_HIGH*
    *DMRES_MEDIUM*
    *DMRES_LOW*
    *DMRES_DRAFT*

    If a positive value is specified, it specifies the number of dots
    per inch (DPI) and is therefore device dependent

and as per wingdi.h, these values are -ve. I have updated the fix to use 
!=0.

>>
>> I have retained defaulting to low 300 dpi resolution as there might 
>> be a case when AwtPrintControl::getDevmode() fails resulting in 
>> returning default values which is -50.
>
> Are you referring to the code in Win32PrintService.java ?
> I'm not sure about doing that. The way I read your code is now if a 
> driver specifies dmQuality as negative
> and leaves yRes at zero (meaning ignore it) .. you now no longer 
> ignore it and return a new PrinterResolution
> for a made up (300,300) resolution.
>
Ok, right. I looked at the spec and saw what you are saying

*dmYResolution*

    Specifies the y-resolution, in dots per inch, of the printer. If the
    printer initializes this member, the *dmPrintQuality* member
    specifies the x-resolution

which means, if dmPrintQuality is initialized +ve for x resolution, the 
y-resolution is specified by dmYResolution.
Or in reverse way, if dmPrintQuality is -ve, then there might not be any 
need of specifying dmYResolution.

I have updated the webrev to cater to both changes

http://cr.openjdk.java.net/~psadhukhan/8186987/webrev.02/

Regards
Prasanta


>> Also, in linux, we do the similar 
>> http://hg.openjdk.java.net/jdk10/client/jdk/file/70359afda5d0/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1565
>>
> What we do in Linux (for Postscript) is different as postscript allows 
> you to specify the DPI for your content
> and the interpreter must handle that.
>
> -phil.
>
>> http://cr.openjdk.java.net/~psadhukhan/8186987/webrev.01/
>>
>> Regards
>> Prasanta
>> [1] 
>> [https://msdn.microsoft.com/en-us/library/windows/desktop/dd183576(v=vs.85).aspx] 
>>
>> On 9/4/2017 8:28 AM, Prahalad Kumar Narayanan wrote
>>> Hello Prasanta
>>>
>>> Thanks for the explanation.
>>> Being new to the Printing subsystem, it helped get the context of 
>>> the problem.
>>>
>>> As I understand, the problem is due to getDefaultPrinterSetings() 
>>> returning negative values for defYRes and quality.
>>> And the fix evades such negative values with a hardcoded low 300 DPI 
>>> resolution.
>>>
>>> I 'm suspecting the bug to be in the code that returns values for 
>>> default printer settings
>>> . Since these values are device specific, I believe, the code might 
>>> use few platform APIs to query the resolutions on that printer 
>>> device & return the same.
>>> . It's here that default resolutions are not retrieved properly.
>>> . So my view is to trace this location & fix the population of 
>>> default printer settings than a hardcoded DPI resolution.
>>>      . When a problem has surfaced on one printer, there is 
>>> possibility for the same to occur on many devices as well.
>>>      . Besides printers may not be supporting low 300 DPI resolution 
>>> going forward.
>>>
>>> I may be wrong in my understanding. You could wait for other's 
>>> review & follow up.
>>>
>>> Thank you
>>> Have a good day
>>>
>>> Prahalad N.
>>>
>>> -----Original Message-----
>>> From: Prasanta Sadhukhan
>>> Sent: Thursday, August 31, 2017 3:39 PM
>>> To: Philip Race; 2d-dev
>>> Subject: [OpenJDK 2D-Dev] [10] RFR JDK-8186987:NullPointerException 
>>> in RasterPrinterJob without PrinterResolution
>>>
>>> Hi All,
>>>
>>> Please review a fix for an issue where it a NPE is seen when an 
>>> attempt is made to print to Brother HL-2240D series printer.
>>>
>>> It seems when RasterPrinterJob#setAttributes() is called with no 
>>> PrinterResolution attribute set, it first checks if 
>>> PrinterResolution category is supported.
>>> If it is supported, then it sees if the supplied resolution value is 
>>> supported. Now, since no PrinterResolution attribute is set, so
>>> isSupportedValue() returns false [as "printer resolution attribute"
>>> object is null]
>>> It then goes to get the default resolution attribute via
>>> getDefaultAttributeValue() which calls getDefaultPrinterSettings() 
>>> and use yRes,Quality from this printer to construct a 
>>> "PrinterResolution"
>>> object.
>>>
>>> Now, it seems in Brother HL-2240D series printer, it supports 3 
>>> resolution [300, 600, HQ 1200] but for all these 3 resolutions,
>>> getDefaultPrinterSettings() returns -50 for yRes and Quality.
>>> So, as per this code
>>> http://hg.openjdk.java.net/jdk10/client/jdk/file/dbb5b171a16b/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1189 
>>>
>>> res < 0 and no PrinterResolution object is instantiated so when RPJ 
>>> accesses 
>>> printerResAttr.getCrossFeedResolution(ResolutionSyntax.DPI); it 
>>> causes NPE.
>>>
>>> Proposed fix is to create a default lowly 300 dpi PrinterResolution 
>>> if, for some reason, yRes and Quality from printer comes out -ve.
>>> http://cr.openjdk.java.net/~psadhukhan/8186987/webrev.00/
>>>
>>> Regards
>>> Prasanta
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20170920/bb09002f/attachment.html>