[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [9] RFR JDK-6357887: selected printertray is ignored under linux
From:       Vadim Pakhnushev <vadim.pakhnushev () oracle ! com>
Date:       2016-08-25 17:14:52
Message-ID: 8669d23b-94f4-ecc2-f672-2fb84d4609f6 () oracle ! com
[Download RAW message or body]

You see, you now have
  102         MediaTray mt = null;
  114         return( mt);

This can be just return null;
No need for new webrev for that, +1.

Vadim

On 25.08.2016 19:14, Prasanta Sadhukhan wrote:
> 
> 
> On 8/25/2016 7:29 PM, Vadim Pakhnushev wrote:
> > In the PSPrinterJob.java customTray.getChoiceName() is not checked 
> > for null but in the UnixPrintJob it is checked.
> > Could this be a problem?
> > 
> Checked for null just to be safe and simplified test method
> Modified webrev
> http://cr.openjdk.java.net/~psadhukhan/6357887/webrev.03/
> 
> Regards
> Prasanta
> > The getMediaTray method in the test could be simplified to
> > for (Media m : media) {
> > if (m instanceof MediaTray) {
> > if (m.toString().trim()...) {
> > return (MediaTray)m;
> > }
> > }
> > }
> > return null;
> > 
> > Vadim
> > 
> > On 25.08.2016 16:25, Philip Race wrote:
> > > +1
> > > 
> > > -phil
> > > 
> > > On 8/24/16, 11:27 PM, Prasanta Sadhukhan wrote:
> > > > Thanks Phil for the comments.
> > > > Modified webrev:
> > > > http://cr.openjdk.java.net/~psadhukhan/6357887/webrev.02/
> > > > 
> > > > Regards
> > > > Prasanta
> > > > On 8/25/2016 12:05 AM, Phil Race wrote:
> > > > > In fact what you should be doing here is
> > > > > Attribute attr = attrs.get(Media.class);
> > > > > if (attr instanceof CustomMediaTray) ....
> > > > > 
> > > > > The program below should show that the lookup works so long as the 
> > > > > key
> > > > > is the class understood by the API - not a sub-type.
> > > > > 
> > > > > -phil.
> > > > > import javax.print.*;
> > > > > import javax.print.attribute.*;
> > > > > import javax.print.attribute.standard.*;
> > > > > 
> > > > > public class CMT {
> > > > > 
> > > > > public static void main(String args[]) throws Exception {
> > > > > 
> > > > > PrintService svc = 
> > > > > PrintServiceLookup.lookupPrintServices(null,null)[0];
> > > > > Media[] allMedia =
> > > > > (Media[])svc.getSupportedAttributeValues(Media.class, null, null);
> > > > > PrintRequestAttributeSet aset = new 
> > > > > HashPrintRequestAttributeSet();
> > > > > for (int m=0;m<allMedia.length;m++) {
> > > > > Media in = allMedia[m];
> > > > > aset.add(in);
> > > > > Media out1 = (Media)aset.get(Media.class);
> > > > > System.out.println("Class="+in.getClass()+" in="+in+ " 
> > > > > out="+out1);
> > > > > Media out2 = (Media)aset.get(in.getClass());
> > > > > System.out.println("Class="+in.getClass()+" in="+in+ " 
> > > > > out="+out2);
> > > > > }
> > > > > }
> > > > > 
> > > > > 
> > > > > 
> > > > > On 8/18/2016 11:20 PM, Prasanta Sadhukhan wrote:
> > > > > > 
> > > > > > 
> > > > > > On 8/19/2016 3:36 AM, Philip Race wrote:
> > > > > > > Oh .. right under the covers the map is just a HashMap
> > > > > > > and the key to the map is the class so just decides if these are
> > > > > > > equal class objects. Hmm .. I don't know anymore if that was the 
> > > > > > > original intent
> > > > > > > but we probably should not mess with it right now just for the 
> > > > > > > optimisation.
> > > > > > > But now I am wondering why it has to be CustomMediaTray and not 
> > > > > > > just
> > > > > > > MediaTray ... although I suspect that in the case of CUPS we are 
> > > > > > > not
> > > > > > > ever mapping the media to standard ones, so they are always custom.
> > > > > > Yes, in CUPS 
> > > > > > http://hg.openjdk.java.net/jdk9/client/jdk/file/9f38d4f86e3d/src/java.desktop/unix/classes/sun/print/CUPSPrinter.java#l254
> > > > > >  we instantiate CustomMediaTray (and do not map to standard) so I 
> > > > > > need to check for CustomMediatray to get the tray instance.
> > > > > > 
> > > > > > Regards
> > > > > > Prasanta
> > > > > > > That might be arise as a problem in the case of your other open 
> > > > > > > review regarding
> > > > > > > validating the margins. I'll comment on that in that review but 
> > > > > > > still surely
> > > > > > > any MediaTray is what you would want here but it is probably 
> > > > > > > moot if
> > > > > > > you don't ever get a standard one. Please check into this and 
> > > > > > > confirm
> > > > > > > what I suspect .. or not ...
> > > > > > > 
> > > > > > > -phil.
> > > > > > > 
> > > > > > > On 8/17/16, 11:41 PM, Prasanta Sadhukhan wrote:
> > > > > > > > MediaTray values are bundled with "Media" attribute so calling 
> > > > > > > > attributes.get(CustomMediatray.class) returns null.
> > > > > > > > Modified webrev to get Media attribute and then see if the 
> > > > > > > > attribute is an instance of CustomMediatray.
> > > > > > > > 
> > > > > > > > http://cr.openjdk.java.net/~psadhukhan/6357887/webrev.01/
> > > > > > > > 
> > > > > > > > Regards
> > > > > > > > Prasanta
> > > > > > > > On 8/17/2016 9:09 PM, Philip Race wrote:
> > > > > > > > > This all looks fine although this can be a simple call to
> > > > > > > > > attributes.get(CustomMediaTray.class) can't it ?
> > > > > > > > > 
> > > > > > > > > 502         for (int i=0; i< attrs.length; i++) {
> > > > > > > > > 503             Attribute attr = attrs[i];
> > > > > > > > > 504             try {
> > > > > > > > > 505                 if (attr instanceof CustomMediaTray) {
> > > > > > > > > 506                     CustomMediaTray customTray = 
> > > > > > > > > (CustomMediaTray)attr;
> > > > > > > > > 507                     mOptions = " InputSlot="+ 
> > > > > > > > > customTray.getChoiceName();
> > > > > > > > > 508                 }
> > > > > > > > > 509             } catch (ClassCastException e) {
> > > > > > > > > 
> > > > > > > > > -phil.
> > > > > > > > > 
> > > > > > > > > On 8/10/16, 1:59 AM, Prasanta Sadhukhan wrote:
> > > > > > > > > > Hi All,
> > > > > > > > > > 
> > > > > > > > > > Please review a fix for an issue where it is seen that the 
> > > > > > > > > > selected printer tray is ignored in linux and default 
> > > > > > > > > > standard tray is used for printing.
> > > > > > > > > > 
> > > > > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-6357887
> > > > > > > > > > webrev: 
> > > > > > > > > > http://cr.openjdk.java.net/~psadhukhan/6357887/webrev.00/
> > > > > > > > > > 
> > > > > > > > > > Issue was lpr command needs "-o InputSlot=<trayname>" to 
> > > > > > > > > > select the printertray which was not being passed to lpr 
> > > > > > > > > > command.
> > > > > > > > > > Proposed fix added InputSlot option to lpr command to select 
> > > > > > > > > > the printertray.
> > > > > > > > > > 
> > > > > > > > > > Tested in ubuntu and solaris11.
> > > > > > > > > > 
> > > > > > > > > > Regards
> > > > > > > > > > Prasanta
> > > > > > > > 
> > > > > > 
> > > > > 
> > > > 
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]