[OpenJDK 2D-Dev] [9] RFR: JDK-8138749, , Revisited: PrinterJob.printDialog() does not support multi-mon, always displayed on primary

[prasanta sadhukhan]

Hi Phil,

On 2/3/2016 11:49 PM, Phil Race wrote:
> Hi,
>
> I think it is worth addressing all this at once and I think it can be 
> done
> entirely in RPJ, leaving ServiceUI alone. Ie fix it just for the case 
> where
> RPJ displays the dialog(s). 
RPJ display the dialog only for pageDialog
http://hg.openjdk.java.net/jdk9/client/jdk/file/b9a92b2a8b37/src/java.desktop/share/classes/sun/print/RasterPrinterJob.java#l820
whereas ServiceUI display the printDialog
http://hg.openjdk.java.net/jdk9/client/jdk/file/b9a92b2a8b37/src/java.desktop/share/classes/javax/print/ServiceUI.java#l227

So, are you saying we fix it only for pageDialog and leave out 
printDialog()?

Regards
Prasanta
> When an application calls ServiceUI directly, it
> can decide where it would like the dialog subject to the existing 
> constraint.
>
> The test really should be one test so that the behaviours can be
> tested as one with lower overhead.
>
> -phil.
>
> On 02/02/2016 09:34 PM, prasanta sadhukhan wrote:
>> Hi Phil,
>>
>> On 2/3/2016 6:05 AM, Phil Race wrote:
>>> The RasterPrinterJob changes are now OK however
>>>
>>> - I am not sure I understand how removing the check in ServiceUI
>>> will address the issue about the dialog needing to be entirely 
>>> on-screen.
>>> It reads to me as if now you allow that in preference to having it 
>>> centred.
>>> But I am not sure the old code would have worked in all cases either
>>> since centering on a very small owner window in the corner of the 
>>> screen
>>> would result in the same problem.
>> Since this issue was about showing the dialog on primary vs 
>> secondary, can we address this issue of showing the dialog entirely 
>> on-screen in separate bug id (since this issue is present in primary 
>> screen too if dialog is bigger than top-level window)?
>> The present change in ServiceUI address the issue of showing the 
>> dialog on secondary monitor if dialog is bigger than top-level.
>>> Probably what is needed is some code that moves it just enough to fit.
>>> ie downwards if it is off the top, left if it is off the right and 
>>> so forth ?
>>>
>>> - The two tests do not share the same bug ID. I expected they
>>> should now do so .. and I was further supposing you would merge
>>> the two tests into one.
>> I will change the bug id of 2nd test. Is it ok if I keep these 2 
>> tests separate to make it easier?
>>
>> Regards
>> Prasanta
>>>
>>> -phil.
>>>
>>> On 01/29/2016 01:34 AM, prasanta sadhukhan wrote:
>>>> Hi Phil,
>>>>
>>>> On 1/28/2016 8:22 PM, Philip Race wrote:
>>>>> "in consistent"
>>>>>
>>>>> On 1/21/16, 11:20 PM, prasanta sadhukhan wrote:
>>>>>> Hi Phil,
>>>>>>
>>>>>> I checked with NATIVE print dialog and with other app print 
>>>>>> dialog, they are positioning the dialog a little ahead of 
>>>>>> top-left corner (not at 1/3rd of the window as we are doing now)
>>>>>> so I did the same and positioned the dialog in consistent with 
>>>>>> our NATIVE print dialog.
>>>>>
>>>>> You mean consistently. in consistent means the opposite
>>>>> Please adjust that in the code comment.
>>>>>
>>>>> 50,50 is probably OK so long as you verified this is approximately
>>>>> what happens on most platforms, not just windows 8 or similar.
>>>>> I don't think you will find complete consistency because apps may
>>>>> control it and use different dialogs even on the same platform.
>>>>> On hidpi screens this may be closer to the top-left than you intend
>>>>> but I expect it is fine.
>>>>>
>>>> Tested on windows, linux and it works ok. We showed the dialog at 
>>>> x+50,y+50 where x,y is the coordinate of the active window.
>>>> Do not have local mac system to test multimonitor setup.
>>>>> The one main thing that you should verify is what happens in the case
>>>>> a native app top-level window is smaller than the print dialog and 
>>>>> is positioned
>>>>> at the bottom right (of the right-most display). Does the native 
>>>>> app ensure
>>>>> that the dialog is wholly on-screen ? I don't think your code will 
>>>>> do that.
>>>>> And Safari on Mac at least seems to do that. I guess that most 
>>>>> apps try
>>>>> their best to keep the dialog wholly on-screen.
>>>>>
>>>> Tested this case where window bounds is smaller than print dialog
>>>> but because of this check in ServiceUI.java
>>>>
>>>> // if portion of dialog is not within the gc boundary
>>>>         if (!gcBounds.contains(dlgBounds)) {
>>>>             // put in the center relative to parent frame/dialog
>>>>             dialog.setLocationRelativeTo(owner);
>>>>         }
>>>>
>>>> the dialog is shown in the primary window.
>>>> I saw we did not have this check for pageDialog(attr) where the 
>>>> dialog is shown in secondary monitor despite gcBounds smaller than 
>>>> print dialog.
>>>> Therefore, I removed this check from ServiceUI.
>>>>
>>>> Please find the updated webrev:
>>>> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.03/
>>>>
>>>> Regards
>>>> Prasanta
>>>>> -phil.
>>>>>
>>>>>
>>>>>> Please find the updated webrev
>>>>>> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.02/
>>>>>>
>>>>>> Regards
>>>>>> Prasanta
>>>>>> On 1/22/2016 4:59 AM, Phil Race wrote:
>>>>>>> That is definitely better .. although I wonder if it would be 
>>>>>>> even better
>>>>>>> if you found when there is an 'active' window that you tried to
>>>>>>> position the dialog such that it is near the upper-left corner 
>>>>>>> of that window ?
>>>>>>> Please experiment and see how that looks compared to what other 
>>>>>>> apps are doing.
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>> On 01/21/2016 01:45 AM, prasanta sadhukhan wrote:
>>>>>>>> Hi Phil,
>>>>>>>>
>>>>>>>> Please find the updated webrev
>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.01/
>>>>>>>> which do away with the change in ServiceUI.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Prasanta
>>>>>>>> On 1/21/2016 2:59 AM, Phil Race wrote:
>>>>>>>>> The changes in ServiceUI seem like they can cause the spec. to 
>>>>>>>>> be contradicted.
>>>>>>>>> It says
>>>>>>>>>
>>>>>>>>>    * @param gc used to select screen. null means primary or 
>>>>>>>>> default screen.
>>>>>>>>>
>>>>>>>>> but you update the code such that it will use the active window.
>>>>>>>>> If that is on a secondary screen but null was passed in.
>>>>>>>>>
>>>>>>>>> I am not entirely sure why ServiceUI needs to be changed at all.
>>>>>>>>> It is just the caller of ServiceUI ..
>>>>>>>>>
>>>>>>>>> -phil.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 01/20/2016 03:08 AM, prasanta sadhukhan wrote:
>>>>>>>>>> Hi All,
>>>>>>>>>>
>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8138749
>>>>>>>>>> webrev: 
>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.00/
>>>>>>>>>>
>>>>>>>>>> Please review a fix for a long standing issue whereby it is 
>>>>>>>>>> seen that
>>>>>>>>>> PrinterJob.printDialog(attr set) does not support 
>>>>>>>>>> multi-monitor setup. When this API is invoked, the print 
>>>>>>>>>> dialog is always displayed on the default screen device 
>>>>>>>>>> regardless of where the application is running.
>>>>>>>>>> This is because this method
>>>>>>>>>> uses ServiceDialog class for creating the dialog and that 
>>>>>>>>>> indeed supports passing a GC in which we would like to have 
>>>>>>>>>> the dialog. But printer job always uses the GraphicsConfig of 
>>>>>>>>>> the default screen device
>>>>>>>>>> resulting in print dialog to be shown on primary device/monitor.
>>>>>>>>>>
>>>>>>>>>> I have not considered pageDialog() for this fix. Will create 
>>>>>>>>>> a separate bugid and send a patch for that as well once this 
>>>>>>>>>> fix is approved.
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Prasanta
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>>
>